From 43b35a12cecd30bba5818f6c00dddddbd42d4cbf Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Tue, 14 Jul 2015 09:31:42 +1000 Subject: [PATCH 01/10] Add models and various shortcuts to allow for AleaIdField with primary_key=True. --- dddp/admin.py | 2 +- dddp/api.py | 22 ++++++++---- dddp/models.py | 95 ++++++++++++++++++++++++++++++++++++++++++++++---- 3 files changed, 105 insertions(+), 14 deletions(-) diff --git a/dddp/admin.py b/dddp/admin.py index ab21e4f..917c078 100644 --- a/dddp/admin.py +++ b/dddp/admin.py @@ -104,7 +104,7 @@ class SubscriptionCollection(admin.ModelAdmin): for name, attr in vars(models).items(): - if hasattr(attr, '_meta'): + if hasattr(attr, '_meta') and not getattr(attr._meta, 'abstract'): model_admin = locals().get(name, None) if model_admin is not False: admin.site.register(attr, model_admin) diff --git a/dddp/api.py b/dddp/api.py index d9d1e54..683bdc6 100644 --- a/dddp/api.py +++ b/dddp/api.py @@ -28,7 +28,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'"}} @@ -600,9 +602,12 @@ 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 + 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) @@ -615,9 +620,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/models.py b/dddp/models.py index 8eea57d..2bf5df7 100644 --- a/dddp/models.py +++ b/dddp/models.py @@ -23,10 +23,29 @@ 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 + 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) + 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,6 +66,13 @@ def get_meteor_id(obj_or_model, obj_pk=None): 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 @@ -59,11 +85,10 @@ def get_meteor_ids(model, object_ids): 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=meteor_random_id('/collection/%s' % meta), ).meteor_id return result @@ -71,10 +96,32 @@ def get_meteor_ids(model, object_ids): @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.") - # Django model._meta is now public API -> pylint: disable=W0212 + + 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. + return model.objects.values_list( + 'pk', flat=True, + ).get(**{ + alea_unique_fields[0].attname: meteor_id, + }) + content_type = ContentType.objects.get_for_model(model) return ObjectMapping.objects.filter( content_type=content_type, @@ -85,6 +132,12 @@ def get_object_id(model, meteor_id): @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) + return model.objects.filter(*args, **kwargs).get( pk=get_object_id(model, meteor_id), ) @@ -97,11 +150,41 @@ class AleaIdField(models.CharField): def __init__(self, *args, **kwargs): """Assume max_length of 17 to match Meteor implementation.""" kwargs.update( - default=meteor_random_id, + editable=False, max_length=17, ) super(AleaIdField, self).__init__(*args, **kwargs) + def deconstruct(self): + """Return details on how this field was defined.""" + name, path, args, kwargs = super(AleaIdField, self).deconstruct() + del kwargs['max_length'] + return name, path, args, kwargs + + def pre_save(self, model_instance, add): + """Generate ID if required.""" + _, _, _, kwargs = self.deconstruct() + val = getattr(model_instance, self.attname) + if val is None and kwargs.get('default', None) is None: + val = meteor_random_id('/collection/%s' % model_instance._meta) + setattr(model_instance, self.attname, val) + return val + + +class AleaIdMixin(models.Model): + + """Django model mixin that provides AleaIdField field (as _id).""" + + id = AleaIdField( + primary_key=True, + ) + + class Meta(object): + + """Model meta options for AleaIdMixin.""" + + abstract = True + @python_2_unicode_compatible class ObjectMapping(models.Model): From aa89ead1eda1fa7d154f253ced99a89298c402e2 Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Thu, 23 Jul 2015 11:07:20 +1000 Subject: [PATCH 02/10] Allow support for multiple meteor apps in a single django-ddp enabled project by reading METEOR_STAR_JSON as part of view init instead of app ready. --- dddp/server/apps.py | 171 ------------------------------------------ dddp/server/views.py | 175 +++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 168 insertions(+), 178 deletions(-) diff --git a/dddp/server/apps.py b/dddp/server/apps.py index 3065be9..4a742e9 100644 --- a/dddp/server/apps.py +++ b/dddp/server/apps.py @@ -1,37 +1,9 @@ """Django DDP Server app config.""" from __future__ import print_function, absolute_import, unicode_literals -import io import mimetypes -import os.path from django.apps import AppConfig -from django.conf import settings -from django.core.exceptions import ImproperlyConfigured -from ejson import dumps, loads -import pybars - - -STAR_JSON_SETTING_NAME = 'METEOR_STAR_JSON' - - -def read(path, default=None, encoding='utf8'): - """Read encoded contents from specified path or return default.""" - if not path: - return default - try: - with io.open(path, mode='r', encoding=encoding) as contents: - return contents.read() - except IOError: - if default is not None: - return default - raise - - -def read_json(path): - """Read JSON encoded contents from specified path.""" - with open(path, mode='r') as json_file: - return loads(json_file.read()) class ServerConfig(AppConfig): @@ -41,149 +13,6 @@ class ServerConfig(AppConfig): name = 'dddp.server' verbose_name = 'Django DDP Meteor Web Server' - manifest = None - program_json = None - program_json_path = None - runtime_config = None - - star_json = None # top level layout - server_json = None # web server layout - web_browser_json = None # web.browser (client) layout - - url_map = None - internal_map = None - server_load_map = None - template_path = None # web server HTML template path - client_map = None # web.browser (client) URL to path map - html = '\nDDP App' - def ready(self): """Configure Django DDP server app.""" mimetypes.init() # read and process /etc/mime.types - self.url_map = {} - - try: - json_path = getattr(settings, STAR_JSON_SETTING_NAME) - except AttributeError: - raise ImproperlyConfigured( - '%s setting required by dddp.server.view.' % ( - STAR_JSON_SETTING_NAME, - ), - ) - - self.star_json = read_json(json_path) - star_format = self.star_json['format'] - if star_format != 'site-archive-pre1': - raise ValueError( - 'Unknown Meteor star format: %r' % star_format, - ) - programs = { - program['name']: program - for program in self.star_json['programs'] - } - - server_json_path = os.path.join( - os.path.dirname(json_path), - os.path.dirname(programs['server']['path']), - 'program.json', - ) - self.server_json = read_json(server_json_path) - server_format = self.server_json['format'] - if server_format != 'javascript-image-pre1': - raise ValueError( - 'Unknown Meteor server format: %r' % server_format, - ) - self.server_load_map = {} - for item in self.server_json['load']: - item['path_full'] = os.path.join( - os.path.dirname(server_json_path), - item['path'], - ) - self.server_load_map[item['path']] = item - self.url_map[item['path']] = ( - item['path_full'], 'text/javascript' - ) - try: - item['source_map_full'] = os.path.join( - os.path.dirname(server_json_path), - item['sourceMap'], - ) - self.url_map[item['sourceMap']] = ( - item['source_map_full'], 'text/plain' - ) - except KeyError: - pass - self.template_path = os.path.join( - os.path.dirname(server_json_path), - self.server_load_map[ - 'packages/boilerplate-generator.js' - ][ - 'assets' - ][ - 'boilerplate_web.browser.html' - ], - ) - - web_browser_json_path = os.path.join( - os.path.dirname(json_path), - programs['web.browser']['path'], - ) - self.web_browser_json = read_json(web_browser_json_path) - web_browser_format = self.web_browser_json['format'] - if web_browser_format != 'web-program-pre1': - raise ValueError( - 'Unknown Meteor web.browser format: %r' % ( - web_browser_format, - ), - ) - self.client_map = {} - self.internal_map = {} - for item in self.web_browser_json['manifest']: - item['path_full'] = os.path.join( - os.path.dirname(web_browser_json_path), - item['path'], - ) - if item['where'] == 'client': - if '?' in item['url']: - item['url'] = item['url'].split('?', 1)[0] - self.client_map[item['url']] = item - self.url_map[item['url']] = ( - item['path_full'], - mimetypes.guess_type( - item['path_full'], - )[0] or 'application/octet-stream', - ) - elif item['where'] == 'internal': - self.internal_map[item['type']] = item - - config = { - 'css': [ - {'url': item['path']} - for item in self.web_browser_json['manifest'] - if item['type'] == 'css' and item['where'] == 'client' - ], - 'js': [ - {'url': item['path']} - for item in self.web_browser_json['manifest'] - if item['type'] == 'js' and item['where'] == 'client' - ], - 'meteorRuntimeConfig': '"%s"' % ( - dumps(self.runtime_config) - ), - 'rootUrlPathPrefix': '/app', - 'bundledJsCssPrefix': '/app/', - 'inlineScriptsAllowed': False, - 'inline': None, - 'head': read( - self.internal_map.get('head', {}).get('path_full', None), - default=u'', - ), - 'body': read( - self.internal_map.get('body', {}).get('path_full', None), - default=u'', - ), - } - tmpl_raw = read(self.template_path, encoding='utf8') - compiler = pybars.Compiler() - tmpl = compiler.compile(tmpl_raw) - self.html = '\n%s' % tmpl(config) diff --git a/dddp/server/views.py b/dddp/server/views.py index 59e4575..9084fc5 100644 --- a/dddp/server/views.py +++ b/dddp/server/views.py @@ -1,13 +1,35 @@ """Django DDP Server views.""" from __future__ import print_function, absolute_import, unicode_literals -from ejson import dumps -from django.apps import apps + +import io +import mimetypes +import os.path + +from ejson import dumps, loads from django.conf import settings from django.http import HttpResponse from django.views.generic import View +import pybars -STAR_JSON_SETTING_NAME = 'METEOR_STAR_JSON' + +def read(path, default=None, encoding='utf8'): + """Read encoded contents from specified path or return default.""" + if not path: + return default + try: + with io.open(path, mode='r', encoding=encoding) as contents: + return contents.read() + except IOError: + if default is not None: + return default + raise + + +def read_json(path): + """Read JSON encoded contents from specified path.""" + with open(path, mode='r') as json_file: + return loads(json_file.read()) class MeteorView(View): @@ -16,13 +38,152 @@ class MeteorView(View): http_method_names = ['get', 'head'] - app = None + json_path = None runtime_config = None + manifest = None + program_json = None + program_json_path = None + runtime_config = None + + star_json = None # top level layout + + url_map = None + internal_map = None + server_load_map = None + template_path = None # web server HTML template path + client_map = None # web.browser (client) URL to path map + html = '\nDDP App' + + root_url_path_prefix = '' + bundled_js_css_prefix = '/' + def __init__(self, **kwargs): """Initialisation for Django DDP server view.""" + # super(...).__init__ assigns kwargs to instance. super(MeteorView, self).__init__(**kwargs) - self.app = apps.get_app_config('server') + + self.url_map = {} + + # process `star_json` + self.star_json = read_json(self.json_path) + star_format = self.star_json['format'] + if star_format != 'site-archive-pre1': + raise ValueError( + 'Unknown Meteor star format: %r' % star_format, + ) + programs = { + program['name']: program + for program in self.star_json['programs'] + } + + # process `bundle/programs/server/program.json` from build dir + server_json_path = os.path.join( + os.path.dirname(self.json_path), + os.path.dirname(programs['server']['path']), + 'program.json', + ) + server_json = read_json(server_json_path) + server_format = server_json['format'] + if server_format != 'javascript-image-pre1': + raise ValueError( + 'Unknown Meteor server format: %r' % server_format, + ) + self.server_load_map = {} + for item in server_json['load']: + item['path_full'] = os.path.join( + os.path.dirname(server_json_path), + item['path'], + ) + self.server_load_map[item['path']] = item + self.url_map[item['path']] = ( + item['path_full'], 'text/javascript' + ) + try: + item['source_map_full'] = os.path.join( + os.path.dirname(server_json_path), + item['sourceMap'], + ) + self.url_map[item['sourceMap']] = ( + item['source_map_full'], 'text/plain' + ) + except KeyError: + pass + self.template_path = os.path.join( + os.path.dirname(server_json_path), + self.server_load_map[ + 'packages/boilerplate-generator.js' + ][ + 'assets' + ][ + 'boilerplate_web.browser.html' + ], + ) + + # process `bundle/programs/web.browser/program.json` from build dir + web_browser_json_path = os.path.join( + os.path.dirname(self.json_path), + programs['web.browser']['path'], + ) + web_browser_json = read_json(web_browser_json_path) + web_browser_format = web_browser_json['format'] + if web_browser_format != 'web-program-pre1': + raise ValueError( + 'Unknown Meteor web.browser format: %r' % ( + web_browser_format, + ), + ) + self.client_map = {} + self.internal_map = {} + for item in web_browser_json['manifest']: + item['path_full'] = os.path.join( + os.path.dirname(web_browser_json_path), + item['path'], + ) + if item['where'] == 'client': + if '?' in item['url']: + item['url'] = item['url'].split('?', 1)[0] + self.client_map[item['url']] = item + self.url_map[item['url']] = ( + item['path_full'], + mimetypes.guess_type( + item['path_full'], + )[0] or 'application/octet-stream', + ) + elif item['where'] == 'internal': + self.internal_map[item['type']] = item + + config = { + 'css': [ + {'url': item['path']} + for item in web_browser_json['manifest'] + if item['type'] == 'css' and item['where'] == 'client' + ], + 'js': [ + {'url': item['path']} + for item in web_browser_json['manifest'] + if item['type'] == 'js' and item['where'] == 'client' + ], + 'meteorRuntimeConfig': '"%s"' % ( + dumps(self.runtime_config) + ), + 'rootUrlPathPrefix': self.root_url_path_prefix, + 'bundledJsCssPrefix': self.bundled_js_css_prefix, + 'inlineScriptsAllowed': False, + 'inline': None, + 'head': read( + self.internal_map.get('head', {}).get('path_full', None), + default=u'', + ), + 'body': read( + self.internal_map.get('body', {}).get('path_full', None), + default=u'', + ), + } + tmpl_raw = read(self.template_path, encoding='utf8') + compiler = pybars.Compiler() + tmpl = compiler.compile(tmpl_raw) + self.html = '\n%s' % tmpl(config) def get(self, request, path): """Return HTML (or other related content) for Meteor.""" @@ -46,7 +207,7 @@ class MeteorView(View): content_type='text/javascript', ) try: - file_path, content_type = self.app.url_map[path] + file_path, content_type = self.url_map[path] with open(file_path, 'r') as content: return HttpResponse( content.read(), @@ -54,5 +215,5 @@ class MeteorView(View): ) except KeyError: print(path) - return HttpResponse(self.app.html) + return HttpResponse(self.html) # raise Http404 From 7663405702f9bc9531740bd11914d8ab3d43393f Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Tue, 11 Aug 2015 09:44:54 +1000 Subject: [PATCH 03/10] Honour AleaIdField(max_length=...) when generating IDs. --- dddp/__init__.py | 4 ++-- dddp/models.py | 36 +++++++++++++++++++++--------------- 2 files changed, 23 insertions(+), 17 deletions(-) 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/models.py b/dddp/models.py index c5d5b78..b812a5c 100644 --- a/dddp/models.py +++ b/dddp/models.py @@ -4,6 +4,7 @@ from __future__ import absolute_import import collections 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, @@ -173,26 +174,31 @@ class AleaIdField(models.CharField): def __init__(self, *args, **kwargs): """Assume max_length of 17 to match Meteor implementation.""" - kwargs.update( - editable=False, - max_length=17, - ) + kwargs.setdefault('editable', False) + kwargs.setdefault('max_length', 17) super(AleaIdField, self).__init__(*args, **kwargs) - def deconstruct(self): - """Return details on how this field was defined.""" - name, path, args, kwargs = super(AleaIdField, self).deconstruct() - del kwargs['max_length'] - return name, path, 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.""" - _, _, _, kwargs = self.deconstruct() - val = getattr(model_instance, self.attname) - if val is None and kwargs.get('default', None) is None: - val = meteor_random_id('/collection/%s' % model_instance._meta) - setattr(model_instance, self.attname, val) - return val + value = super(AleaIdField, self).pre_save(model_instance, add) + if (not value) and self.default is NOT_PROVIDED: + value = self.get_seeded_value(model_instance) + setattr(model_instance, self.attname, value) + return value class AleaIdMixin(models.Model): From 5939d47af634ae47de34764afec881f5af4d40a0 Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Tue, 11 Aug 2015 09:48:00 +1000 Subject: [PATCH 04/10] Normalise paths in dddp.server.views before comparison. --- dddp/server/views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dddp/server/views.py b/dddp/server/views.py index 9084fc5..318cfb3 100644 --- a/dddp/server/views.py +++ b/dddp/server/views.py @@ -187,6 +187,8 @@ class MeteorView(View): def get(self, request, path): """Return HTML (or other related content) for Meteor.""" + if path[:1] != '/': + path = '/%s' % path if path == '/meteor_runtime_config.js': config = { 'DDP_DEFAULT_CONNECTION_URL': request.build_absolute_uri('/'), From f02df498a2f2181518def35b3a4881283c154d43 Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Wed, 12 Aug 2015 12:02:56 +1000 Subject: [PATCH 05/10] Remove console noise from logging handler init. --- dddp/logging.py | 1 - 1 file changed, 1 deletion(-) 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) From b32e5aa54798aa7229e9cf2038fe31e272b22436 Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Thu, 13 Aug 2015 09:43:38 +1000 Subject: [PATCH 06/10] Add missing imports --- dddp/views.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dddp/views.py b/dddp/views.py index a631b95..76ebfac 100644 --- a/dddp/views.py +++ b/dddp/views.py @@ -1,7 +1,9 @@ """Django DDP Server views.""" from __future__ import print_function, absolute_import, unicode_literals +import io import mimetypes +import os.path from ejson import dumps, loads from django.conf import settings From 489175d9a97f8d1538835e6a36b8a91e45a48a5f Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Thu, 13 Aug 2015 09:44:45 +1000 Subject: [PATCH 07/10] Added helper for migrations involving AleaIdField to populate fields from ObjectMapping table. --- dddp/migrations/__init__.py | 48 +++++++++++++++++++++++++++++++++++++ 1 file changed, 48 insertions(+) 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 From 04ef1ab8a025751e0f6ab79b94f002ce5ad64753 Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Thu, 13 Aug 2015 09:46:31 +1000 Subject: [PATCH 08/10] 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): From 7c54f4c324c8c664ceed6219ebd5cb5acc7e526c Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Thu, 13 Aug 2015 09:47:16 +1000 Subject: [PATCH 09/10] Added migration for changed field defaults on Connection.connection_id. --- dddp/migrations/0009_auto_20150812_0856.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) create mode 100644 dddp/migrations/0009_auto_20150812_0856.py 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'), + ), + ] From b1017c78e5089da4b504b4f8f6c221a1ee05e676 Mon Sep 17 00:00:00 2001 From: Tyson Clugg Date: Thu, 13 Aug 2015 10:00:54 +1000 Subject: [PATCH 10/10] Update CHANGES.rst, bump version number. --- CHANGES.rst | 9 +++++++++ setup.py | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) 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/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',