From 56651a154eee0668044eeeaf0733d1b05c7f4366 Mon Sep 17 00:00:00 2001 From: Marc Date: Fri, 18 Jul 2014 15:32:27 +0000 Subject: [PATCH] Improvements on orders --- TODO.md | 5 ++ orchestra/admin/utils.py | 4 +- orchestra/apps/miscellaneous/admin.py | 7 ++ orchestra/apps/miscellaneous/models.py | 3 + orchestra/apps/orders/admin.py | 3 +- orchestra/apps/orders/helpers.py | 38 ++++++++++ orchestra/apps/orders/middlewares.py | 79 --------------------- orchestra/apps/orders/models.py | 97 ++++++++++++++++++-------- orchestra/apps/resources/admin.py | 2 +- orchestra/apps/resources/forms.py | 2 - orchestra/apps/resources/models.py | 12 +++- orchestra/apps/users/roles/admin.py | 10 +-- orchestra/bin/orchestra-admin | 2 +- orchestra/conf/base_settings.py | 1 - 14 files changed, 143 insertions(+), 122 deletions(-) create mode 100644 orchestra/apps/orders/helpers.py delete mode 100644 orchestra/apps/orders/middlewares.py diff --git a/TODO.md b/TODO.md index 075fe2b7..4f1eb6f8 100644 --- a/TODO.md +++ b/TODO.md @@ -57,3 +57,8 @@ Remember that, as always with QuerySets, any subsequent chained methods which im * Settings dictionary like DRF2 in order to better override large settings like WEBSITES_APPLICATIONS.etc + + +* DOCUMENT: orchestration.middleware: we need to know when an operation starts and ends in order to perform bulk server updates and also to wait for related objects to be saved (base object is saved first and then related) + orders.signales: we perform changes right away because data model state can change under monitoring and other periodik task, and we should keep orders consistency under any situation. + dependency collector with max_recursion that matches the number of dots on service.match and service.metric diff --git a/orchestra/admin/utils.py b/orchestra/admin/utils.py index aee8e478..2e7a00b7 100644 --- a/orchestra/admin/utils.py +++ b/orchestra/admin/utils.py @@ -17,7 +17,7 @@ def get_modeladmin(model, import_module=True): """ returns the modeladmin registred for model """ for k,v in admin.site._registry.iteritems(): if k is model: - return type(v) + return v if import_module: # Sometimes the admin module is not yet imported app_label = model._meta.app_label @@ -32,7 +32,7 @@ def insertattr(model, name, value, weight=0): """ Inserts attribute to a modeladmin """ modeladmin = model if models.Model in model.__mro__: - modeladmin = get_modeladmin(model) + modeladmin = type(get_modeladmin(model)) # Avoid inlines defined on parent class be shared between subclasses # Seems that if we use tuples they are lost in some conditions like changing # the tuple in modeladmin.__init__ diff --git a/orchestra/apps/miscellaneous/admin.py b/orchestra/apps/miscellaneous/admin.py index d8f9eccd..dc623b1a 100644 --- a/orchestra/apps/miscellaneous/admin.py +++ b/orchestra/apps/miscellaneous/admin.py @@ -28,6 +28,13 @@ class MiscServiceAdmin(admin.ModelAdmin): class MiscellaneousAdmin(AccountAdminMixin, admin.ModelAdmin): list_display = ('service', 'amount', 'account_link') + + def get_fields(self, request, obj=None): + if obj is None: + return ('service', 'account', 'description', 'amount', 'is_active') + if not obj.service.has_amount: + return ('service', 'account_link', 'description', 'is_active') + return ('service', 'account_link', 'description', 'amount', 'is_active') admin.site.register(MiscService, MiscServiceAdmin) diff --git a/orchestra/apps/miscellaneous/models.py b/orchestra/apps/miscellaneous/models.py index 6bb4c83d..2d76ab78 100644 --- a/orchestra/apps/miscellaneous/models.py +++ b/orchestra/apps/miscellaneous/models.py @@ -7,6 +7,9 @@ from orchestra.core import services class MiscService(models.Model): name = models.CharField(_("name"), max_length=256) description = models.TextField(blank=True) + has_amount = models.BooleanField(default=False, + help_text=_("Designates whether this service has amount " + "property or not.")) is_active = models.BooleanField(default=True, help_text=_("Whether new instances of this service can be created " "or not. Unselect this instead of deleting services.")) diff --git a/orchestra/apps/orders/admin.py b/orchestra/apps/orders/admin.py index 99f83e3f..c69bf38e 100644 --- a/orchestra/apps/orders/admin.py +++ b/orchestra/apps/orders/admin.py @@ -2,6 +2,7 @@ from django import forms from django.contrib import admin from django.utils.translation import ugettext_lazy as _ +from orchestra.apps.accounts.admin import AccountAdminMixin from orchestra.core import services from .models import Service, Order, MetricStorage @@ -36,7 +37,7 @@ class ServiceAdmin(admin.ModelAdmin): return super(ServiceAdmin, self).formfield_for_dbfield(db_field, **kwargs) -class OrderAdmin(admin.ModelAdmin): +class OrderAdmin(AccountAdminMixin, admin.ModelAdmin): pass diff --git a/orchestra/apps/orders/helpers.py b/orchestra/apps/orders/helpers.py new file mode 100644 index 00000000..746fdfa4 --- /dev/null +++ b/orchestra/apps/orders/helpers.py @@ -0,0 +1,38 @@ +import inspect + +from orchestra.apps.accounts.models import Account + + +def search_for_related(origin, max_depth=2): + """ + Introspects origin object and return the first related service object + + WARNING this is NOT an exhaustive search but a compromise between cost and + flexibility. A more comprehensive approach may be considered if + a use-case calls for it. + """ + + def related_iterator(node): + for field in node._meta.virtual_fields: + if hasattr(field, 'ct_field'): + yield getattr(node, field.name) + for field in node._meta.fields: + if field.rel: + yield getattr(node, field.name) + + # BFS model relation transversal + queue = [[origin]] + while queue: + models = queue.pop(0) + if len(models) > max_depth: + return None + node = models[-1] + if len(models) > 1: + if hasattr(node, 'account') or isinstance(node, Account): + return node + for related in related_iterator(node): + if related not in models: + new_models = list(models) + new_models.append(related) + queue.append(new_models) + diff --git a/orchestra/apps/orders/middlewares.py b/orchestra/apps/orders/middlewares.py deleted file mode 100644 index 7807b81a..00000000 --- a/orchestra/apps/orders/middlewares.py +++ /dev/null @@ -1,79 +0,0 @@ -from threading import local - -from django.db.models.signals import pre_delete, pre_save -from django.dispatch import receiver -from django.http.response import HttpResponseServerError - -from orchestra.core import services -from orchestra.utils.python import OrderedSet - -from .models import Order - - -@receiver(pre_save, dispatch_uid='orders.ppre_save_collector') -def pre_save_collector(sender, *args, **kwargs): - if sender in services: - OrderMiddleware.collect(Order.SAVE, **kwargs) - -@receiver(pre_delete, dispatch_uid='orders.pre_delete_collector') -def pre_delete_collector(sender, *args, **kwargs): - if sender in services: - OrderMiddleware.collect(Order.DELETE, **kwargs) - - -class OrderCandidate(object): - def __unicode__(self): - return "{}.{}()".format(str(self.instance), self.action) - - def __init__(self, instance, action): - self.instance = instance - self.action = action - - def __hash__(self): - """ set() """ - opts = self.instance._meta - model = opts.app_label + opts.model_name - return hash(model + str(self.instance.pk) + self.action) - - def __eq__(self, candidate): - """ set() """ - return hash(self) == hash(candidate) - - -class OrderMiddleware(object): - """ - Stores all the operations derived from save and delete signals and executes them - at the end of the request/response cycle - """ - # Thread local is used because request object is not available on model signals - thread_locals = local() - - @classmethod - def get_order_candidates(cls): - # Check if an error poped up before OrdersMiddleware.process_request() - if hasattr(cls.thread_locals, 'request'): - request = cls.thread_locals.request - if not hasattr(request, 'order_candidates'): - request.order_candidates = OrderedSet() - return request.order_candidates - return set() - - @classmethod - def collect(cls, action, **kwargs): - """ Collects all pending operations derived from model signals """ - request = getattr(cls.thread_locals, 'request', None) - if request is None: - return - order_candidates = cls.get_order_candidates() - instance = kwargs['instance'] - order_candidates.add(OrderCandidate(instance, action)) - - def process_request(self, request): - """ Store request on a thread local variable """ - type(self).thread_locals.request = request - - def process_response(self, request, response): - if not isinstance(response, HttpResponseServerError): - candidates = type(self).get_order_candidates() - Order.process_candidates(candidates) - return response diff --git a/orchestra/apps/orders/models.py b/orchestra/apps/orders/models.py index 5ab2e362..3ef47979 100644 --- a/orchestra/apps/orders/models.py +++ b/orchestra/apps/orders/models.py @@ -1,9 +1,15 @@ from django.db import models +from django.db.models import Q +from django.db.models.signals import pre_delete, post_delete, post_save +from django.dispatch import receiver +from django.contrib.admin.models import LogEntry from django.contrib.contenttypes import generic from django.contrib.contenttypes.models import ContentType +from django.utils import timezone from django.utils.translation import ugettext_lazy as _ from . import settings +from .helpers import search_for_related class Service(models.Model): @@ -31,6 +37,7 @@ class Service(models.Model): model = models.ForeignKey(ContentType, verbose_name=_("model")) match = models.CharField(_("match"), max_length=256) is_active = models.BooleanField(_("is active"), default=True) + # TODO class based Service definition (like ServiceBackend) # Billing billing_period = models.CharField(_("billing period"), max_length=16, help_text=_("Renewal period for recurring invoicing"), @@ -145,6 +152,7 @@ class Service(models.Model): @classmethod def get_services(cls, instance, **kwargs): + # TODO get per-request cache from thread local cache = kwargs.get('cache', {}) ct = ContentType.objects.get_for_model(type(instance)) try: @@ -155,11 +163,23 @@ class Service(models.Model): def matches(self, instance): safe_locals = { - 'instance': instance + instance._meta.model_name: instance } return eval(self.match, safe_locals) +class OrderQuerySet(models.QuerySet): + def by_object(self, obj, *args, **kwargs): + ct = ContentType.objects.get_for_model(obj) + return self.filter(object_id=obj.pk, content_type=ct) + + def active(self, *args, **kwargs): + """ return active orders """ + return self.filter( + Q(cancelled_on__isnull=True) | Q(cancelled_on__gt=timezone.now()) + ).filter(*args, **kwargs) + + class Order(models.Model): SAVE = 'SAVE' DELETE = 'DELETE' @@ -178,7 +198,8 @@ class Order(models.Model): description = models.TextField(_("description"), blank=True) content_object = generic.GenericForeignKey() - + objects = OrderQuerySet.as_manager() + def __unicode__(self): return str(self.service) @@ -193,33 +214,23 @@ class Order(models.Model): self.save() @classmethod - def process_candidates(cls, candidates): - cache = {} - for candidate in candidates: - instance = candidate.instance - if candidate.action == cls.DELETE: - cls.objects.filter_for_object(instance).cancel() - else: - for service in Service.get_services(instance, cache=cache): - print cache - if not instance.pk: - if service.matches(instance): - order = cls.objects.create(content_object=instance, - account_id=instance.account_id, service=service) - order.update() - else: - ct = ContentType.objects.get_for_model(instance) - orders = cls.objects.filter(content_type=ct, service=service, - object_id=instance.pk) - if service.matches(instance): - if not orders: - order = cls.objects.create(content_object=instance, - service=service, account_id=instance.account_id) - else: - order = orders.get() - order.update() - elif orders: - orders.get().cancel() + def update_orders(cls, instance): + for service in Service.get_services(instance): + orders = Order.objects.by_object(instance, service=service).active() + if service.matches(instance): + if not orders: + account_id = getattr(instance, 'account_id', instance.pk) + order = cls.objects.create(content_object=instance, + service=service, account_id=account_id) + else: + order = orders.get() + order.update() + elif orders: + orders.get().cancel() + + def cancel(self): + self.cancelled_on = timezone.now() + self.save() class MetricStorage(models.Model): @@ -229,3 +240,31 @@ class MetricStorage(models.Model): def __unicode__(self): return self.order + + + +@receiver(pre_delete, dispatch_uid="orders.cancel_orders") +def cancel_orders(sender, **kwargs): + if not sender in [MetricStorage, LogEntry, Order, Service]: + instance = kwargs['instance'] + for order in Order.objects.by_object(instance).active(): + order.cancel() + + +@receiver(post_delete, dispatch_uid="orders.update_orders_on_delete") +def update_orders_on_delete(sender, **kwargs): + if not sender in [MetricStorage, LogEntry, Order, Service]: + instance = kwargs['instance'] + related = search_for_related(instance) + if related: + Order.update_orders(related) + + +@receiver(post_save, dispatch_uid="orders.update_orders") +def update_orders(sender, **kwargs): + if not sender in [MetricStorage, LogEntry, Order, Service]: + instance = kwargs['instance'] + Order.update_orders(instance) + related = search_for_related(instance) + if related: + Order.update_orders(related) diff --git a/orchestra/apps/resources/admin.py b/orchestra/apps/resources/admin.py index 542e503f..1f93d2bf 100644 --- a/orchestra/apps/resources/admin.py +++ b/orchestra/apps/resources/admin.py @@ -47,7 +47,7 @@ class ResourceAdmin(ExtendedModelAdmin): def save_model(self, request, obj, form, change): super(ResourceAdmin, self).save_model(request, obj, form, change) model = obj.content_type.model_class() - modeladmin = get_modeladmin(model) + modeladmin = type(get_modeladmin(model)) resources = obj.content_type.resource_set.filter(is_active=True) inlines = [] for inline in modeladmin.inlines: diff --git a/orchestra/apps/resources/forms.py b/orchestra/apps/resources/forms.py index 05145808..7caa529d 100644 --- a/orchestra/apps/resources/forms.py +++ b/orchestra/apps/resources/forms.py @@ -9,8 +9,6 @@ from orchestra.forms.widgets import ShowTextWidget, ReadOnlyWidget class ResourceForm(forms.ModelForm): verbose_name = forms.CharField(label=_("Name"), widget=ShowTextWidget(bold=True), required=False) - used = forms.IntegerField(label=_("Used"), widget=ShowTextWidget(), - required=False) allocated = forms.IntegerField(label=_("Allocated")) unit = forms.CharField(label=_("Unit"), widget=ShowTextWidget(), required=False) diff --git a/orchestra/apps/resources/models.py b/orchestra/apps/resources/models.py index 99b999c9..a1d80e25 100644 --- a/orchestra/apps/resources/models.py +++ b/orchestra/apps/resources/models.py @@ -161,7 +161,17 @@ class MonitorData(models.Model): def create_resource_relation(): + class ResourceHandler(object): + """ account.resources.web """ + def __getattr__(self, attr): + return self.obj.resource_set.get(resource__name=attr) + + def __get__(self, obj, cls): + self.obj = obj + return self + relation = GenericRelation('resources.ResourceData') for resources in Resource.group_by_content_type(): model = resources[0].content_type.model_class() - model.add_to_class('resources', relation) + model.add_to_class('resource_set', relation) + model.resources = ResourceHandler() diff --git a/orchestra/apps/users/roles/admin.py b/orchestra/apps/users/roles/admin.py index 1e442cf3..735e4d72 100644 --- a/orchestra/apps/users/roles/admin.py +++ b/orchestra/apps/users/roles/admin.py @@ -33,10 +33,10 @@ class RoleAdmin(object): return False def get_user(self, request, object_id): - modeladmin = get_modeladmin(User) - user = modeladmin.get_object(request, unquote(object_id)) - opts = self.model._meta - if user is None: + try: + user = User.objects.get(pk=unquote(object_id)) + except User.DoesNotExist: + opts = self.model._meta raise Http404( _('%(name)s object with primary key %(key)r does not exist.') % {'name': force_text(opts.verbose_name), 'key': escape(object_id)} @@ -53,7 +53,7 @@ class RoleAdmin(object): obj = getattr(user, self.name) form_class = self.form if self.form else role_form_factory(self) form = form_class(instance=obj) - opts = modeladmin.model._meta + opts = User._meta app_label = opts.app_label title = _("Add %s for user %s" % (self.name, user)) action = _("Create") diff --git a/orchestra/bin/orchestra-admin b/orchestra/bin/orchestra-admin index d5d5fa33..42638df1 100755 --- a/orchestra/bin/orchestra-admin +++ b/orchestra/bin/orchestra-admin @@ -128,7 +128,7 @@ function install_requirements () { python-cracklib" PIP="django==1.6.1 \ - django-celery-email==1.0.3 \ + django-celery-email==1.0.4 \ django-fluent-dashboard==0.3.5 \ https://bitbucket.org/izi/django-admin-tools/get/a0abfffd76a0.zip \ IPy==0.81 \ diff --git a/orchestra/conf/base_settings.py b/orchestra/conf/base_settings.py index d144bd53..c5789e17 100644 --- a/orchestra/conf/base_settings.py +++ b/orchestra/conf/base_settings.py @@ -43,7 +43,6 @@ MIDDLEWARE_CLASSES = ( 'django.contrib.auth.middleware.AuthenticationMiddleware', 'django.contrib.messages.middleware.MessageMiddleware', 'django.middleware.transaction.TransactionMiddleware', - 'orchestra.apps.orders.middlewares.OrderMiddleware', 'orchestra.apps.orchestration.middlewares.OperationsMiddleware', # Uncomment the next line for simple clickjacking protection: # 'django.middleware.clickjacking.XFrameOptionsMiddleware',