From f7cb067b527ddab656e1b55ce101beda64dab5c0 Mon Sep 17 00:00:00 2001 From: smilerz Date: Tue, 11 Jan 2022 07:24:59 -0600 Subject: [PATCH] construct values in queryset instead of serializer methods --- cookbook/helper/HelperFunctions.py | 2 +- cookbook/serializer.py | 44 ++++++++++---------- cookbook/views/api.py | 65 +++++++++++++++++++++++++----- recipes/middleware.py | 7 ++-- recipes/settings.py | 4 +- 5 files changed, 82 insertions(+), 40 deletions(-) diff --git a/cookbook/helper/HelperFunctions.py b/cookbook/helper/HelperFunctions.py index cf04c3e2..e2971c2e 100644 --- a/cookbook/helper/HelperFunctions.py +++ b/cookbook/helper/HelperFunctions.py @@ -7,7 +7,7 @@ class Round(Func): def str2bool(v): - if type(v) == bool: + if type(v) == bool or v is None: return v else: return v.lower() in ("yes", "true", "1") diff --git a/cookbook/serializer.py b/cookbook/serializer.py index ac623868..652e94f8 100644 --- a/cookbook/serializer.py +++ b/cookbook/serializer.py @@ -12,6 +12,7 @@ from rest_framework import serializers from rest_framework.exceptions import NotFound, ValidationError from rest_framework.fields import empty +from cookbook.helper.HelperFunctions import str2bool from cookbook.helper.shopping_helper import list_from_recipe from cookbook.models import (Automation, BookmarkletImport, Comment, CookLog, Food, FoodInheritField, ImportLog, Ingredient, Keyword, MealPlan, MealType, @@ -21,13 +22,18 @@ from cookbook.models import (Automation, BookmarkletImport, Comment, CookLog, Fo SupermarketCategoryRelation, Sync, SyncLog, Unit, UserFile, UserPreference, ViewLog) from cookbook.templatetags.custom_tags import markdown +from recipes.settings import MEDIA_URL, SCRIPT_NAME class ExtendedRecipeMixin(serializers.ModelSerializer): # adds image and recipe count to serializer when query param extended=1 - image = serializers.SerializerMethodField('get_image') - numrecipe = serializers.SerializerMethodField('count_recipes') + # ORM path to this object from Recipe recipe_filter = None + # list of ORM paths to any image + images = None + + image = serializers.SerializerMethodField('get_image') + numrecipe = serializers.ReadOnlyField(source='count_recipes_test') def get_fields(self, *args, **kwargs): fields = super().get_fields(*args, **kwargs) @@ -37,8 +43,7 @@ class ExtendedRecipeMixin(serializers.ModelSerializer): api_serializer = None # extended values are computationally expensive and not needed in normal circumstances try: - if bool(int( - self.context['request'].query_params.get('extended', False))) and self.__class__ == api_serializer: + if str2bool(self.context['request'].query_params.get('extended', False)) and self.__class__ == api_serializer: return fields except (AttributeError, KeyError) as e: pass @@ -50,21 +55,8 @@ class ExtendedRecipeMixin(serializers.ModelSerializer): return fields def get_image(self, obj): - # TODO add caching - recipes = Recipe.objects.filter(**{self.recipe_filter: obj}, space=obj.space).exclude( - image__isnull=True).exclude(image__exact='') - try: - if recipes.count() == 0 and obj.has_children(): - obj__in = self.recipe_filter + '__in' - recipes = Recipe.objects.filter(**{obj__in: obj.get_descendants()}, space=obj.space).exclude( - image__isnull=True).exclude(image__exact='') # if no recipes found - check whole tree - except AttributeError: - # probably not a tree - pass - if recipes.count() != 0: - return random.choice(recipes).image.url - else: - return None + if obj.recipe_image: + return SCRIPT_NAME + MEDIA_URL + obj.recipe_image def count_recipes(self, obj): return Recipe.objects.filter(**{self.recipe_filter: obj}, space=obj.space).count() @@ -98,7 +90,11 @@ class CustomOnHandField(serializers.Field): return instance def to_representation(self, obj): - shared_users = [x.id for x in list(self.context['request'].user.get_shopping_share())] + [self.context['request'].user.id] + shared_users = [] + if request := self.context.get('request', None): + shared_users = request._shared_users + else: + shared_users = [x.id for x in list(self.context['request'].user.get_shopping_share())] + [self.context['request'].user.id] return obj.onhand_users.filter(id__in=shared_users).exists() def to_internal_value(self, data): @@ -379,14 +375,16 @@ class RecipeSimpleSerializer(serializers.ModelSerializer): class FoodSerializer(UniqueFieldsMixin, WritableNestedModelSerializer, ExtendedRecipeMixin): supermarket_category = SupermarketCategorySerializer(allow_null=True, required=False) recipe = RecipeSimpleSerializer(allow_null=True, required=False) - shopping = serializers.SerializerMethodField('get_shopping_status') + # shopping = serializers.SerializerMethodField('get_shopping_status') + shopping = serializers.ReadOnlyField(source='shopping_status') inherit_fields = FoodInheritFieldSerializer(many=True, allow_null=True, required=False) food_onhand = CustomOnHandField(required=False, allow_null=True) recipe_filter = 'steps__ingredients__food' + images = ['recipe__image'] - def get_shopping_status(self, obj): - return ShoppingListEntry.objects.filter(space=obj.space, food=obj, checked=False).count() > 0 + # def get_shopping_status(self, obj): + # return ShoppingListEntry.objects.filter(space=obj.space, food=obj, checked=False).count() > 0 def create(self, validated_data): validated_data['name'] = validated_data['name'].strip() diff --git a/cookbook/views/api.py b/cookbook/views/api.py index 86f97660..b0023d9c 100644 --- a/cookbook/views/api.py +++ b/cookbook/views/api.py @@ -12,8 +12,9 @@ from django.contrib.auth.models import User from django.contrib.postgres.search import TrigramSimilarity from django.core.exceptions import FieldError, ValidationError from django.core.files import File -from django.db.models import Case, ProtectedError, Q, Value, When +from django.db.models import Case, Count, Exists, OuterRef, ProtectedError, Q, Subquery, Value, When from django.db.models.fields.related import ForeignObjectRel +from django.db.models.functions import Coalesce from django.http import FileResponse, HttpResponse, JsonResponse from django.shortcuts import get_object_or_404, redirect from django.urls import reverse @@ -30,6 +31,7 @@ from rest_framework.response import Response from rest_framework.viewsets import ViewSetMixin from treebeard.exceptions import InvalidMoveToDescendant, InvalidPosition, PathOverflow +from cookbook.helper.HelperFunctions import str2bool from cookbook.helper.image_processing import handle_image from cookbook.helper.ingredient_parser import IngredientParser from cookbook.helper.permission_helper import (CustomIsAdmin, CustomIsGuest, CustomIsOwner, @@ -100,7 +102,38 @@ class DefaultPagination(PageNumberPagination): max_page_size = 200 -class FuzzyFilterMixin(ViewSetMixin): +class ExtendedRecipeMixin(): + ''' + ExtendedRecipe annotates a queryset with recipe_image and recipe_count values + ''' + @classmethod + def annotate_recipe(self, queryset=None, request=None, serializer=None, tree=False): + extended = str2bool(request.query_params.get('extended', None)) + if extended: + recipe_filter = serializer.recipe_filter + images = serializer.images + space = request.space + + # add a recipe count annotation to the query + # explanation on construction https://stackoverflow.com/a/43771738/15762829 + recipe_count = Recipe.objects.filter(**{recipe_filter: OuterRef('id')}, space=space).values(recipe_filter).annotate(count=Count('pk')).values('count') + queryset = queryset.annotate(recipe_count_test=Coalesce(Subquery(recipe_count), 0)) + + # add a recipe image annotation to the query + image_subquery = Recipe.objects.filter(**{recipe_filter: OuterRef('id')}, space=space).exclude(image__isnull=True).exclude(image__exact='').order_by("?").values('image')[:1] + if tree: + image_children_subquery = Recipe.objects.filter(**{f"{recipe_filter}__path__startswith": OuterRef('path')}, + space=space).exclude(image__isnull=True).exclude(image__exact='').order_by("?").values('image')[:1] + else: + image_children_subquery = None + if images: + queryset = queryset.annotate(recipe_image=Coalesce(*images, image_subquery, image_children_subquery)) + else: + queryset = queryset.annotate(recipe_image=Coalesce(image_subquery, image_children_subquery)) + return queryset + + +class FuzzyFilterMixin(ViewSetMixin, ExtendedRecipeMixin): schema = FilterSchema() def get_queryset(self): @@ -141,12 +174,12 @@ class FuzzyFilterMixin(ViewSetMixin): if random: self.queryset = self.queryset.order_by("?") self.queryset = self.queryset[:int(limit)] - return self.queryset + return self.annotate_recipe(queryset=self.queryset, request=self.request, serializer=self.serializer_class) class MergeMixin(ViewSetMixin): - @decorators.action(detail=True, url_path='merge/(?P[^/.]+)', methods=['PUT'], ) - @decorators.renderer_classes((TemplateHTMLRenderer, JSONRenderer)) + @ decorators.action(detail=True, url_path='merge/(?P[^/.]+)', methods=['PUT'], ) + @ decorators.renderer_classes((TemplateHTMLRenderer, JSONRenderer)) def merge(self, request, pk, target): self.description = f"Merge {self.basename} onto target {self.basename} with ID of [int]." @@ -211,7 +244,7 @@ class MergeMixin(ViewSetMixin): return Response(content, status=status.HTTP_400_BAD_REQUEST) -class TreeMixin(MergeMixin, FuzzyFilterMixin): +class TreeMixin(MergeMixin, FuzzyFilterMixin, ExtendedRecipeMixin): schema = TreeSchema() model = None @@ -237,11 +270,13 @@ class TreeMixin(MergeMixin, FuzzyFilterMixin): except self.model.DoesNotExist: self.queryset = self.model.objects.none() else: - return super().get_queryset() - return self.queryset.filter(space=self.request.space).order_by('name') + self.queryset = super().get_queryset() + self.queryset = self.queryset.filter(space=self.request.space).order_by('name') - @decorators.action(detail=True, url_path='move/(?P[^/.]+)', methods=['PUT'], ) - @decorators.renderer_classes((TemplateHTMLRenderer, JSONRenderer)) + return self.annotate_recipe(queryset=self.queryset, request=self.request, serializer=self.serializer_class, tree=True) + + @ decorators.action(detail=True, url_path='move/(?P[^/.]+)', methods=['PUT'], ) + @ decorators.renderer_classes((TemplateHTMLRenderer, JSONRenderer)) def move(self, request, pk, parent): self.description = f"Move {self.basename} to be a child of {self.basename} with ID of [int]. Use ID: 0 to move {self.basename} to the root." if self.model.node_order_by: @@ -413,7 +448,15 @@ class FoodViewSet(viewsets.ModelViewSet, TreeMixin): permission_classes = [CustomIsUser] pagination_class = DefaultPagination - @decorators.action(detail=True, methods=['PUT'], serializer_class=FoodShoppingUpdateSerializer,) + def get_queryset(self): + self.request._shared_users = [x.id for x in list(self.request.user.get_shopping_share())] + [self.request.user.id] + + self.queryset = super().get_queryset() + shopping_status = ShoppingListEntry.objects.filter(space=self.request.space, food=OuterRef('id'), checked=False).values('id') + # onhand_status = self.queryset.annotate(onhand_status=Exists(onhand_users_set__in=[shared_users])) + return self.queryset.annotate(shopping_status=Exists(shopping_status)).prefetch_related('onhand_users', 'inherit_fields').select_related('recipe', 'supermarket_category') + + @ decorators.action(detail=True, methods=['PUT'], serializer_class=FoodShoppingUpdateSerializer,) # TODO DRF only allows one action in a decorator action without overriding get_operation_id_base() this should be PUT and DELETE probably def shopping(self, request, pk): if self.request.space.demo: diff --git a/recipes/middleware.py b/recipes/middleware.py index ebe9c51f..09346e6a 100644 --- a/recipes/middleware.py +++ b/recipes/middleware.py @@ -13,19 +13,20 @@ class CustomRemoteUser(RemoteUserMiddleware): Gist code by vstoykov, you can check his original gist at: https://gist.github.com/vstoykov/1390853/5d2e8fac3ca2b2ada8c7de2fb70c021e50927375 Changes: -Ignoring static file requests and a certain useless admin request from triggering the logger. +Ignoring static file requests and a certain useless admin request from triggering the logger. Updated statements to make it Python 3 friendly. """ - def terminal_width(): """ Function to compute the terminal width. """ width = 0 try: - import struct, fcntl, termios + import fcntl + import struct + import termios s = struct.pack('HHHH', 0, 0, 0, 0) x = fcntl.ioctl(1, termios.TIOCGWINSZ, s) width = struct.unpack('HHHH', x)[1] diff --git a/recipes/settings.py b/recipes/settings.py index 9b9dacbb..dcbbfc07 100644 --- a/recipes/settings.py +++ b/recipes/settings.py @@ -371,10 +371,10 @@ LANGUAGES = [ # Static files (CSS, JavaScript, Images) # https://docs.djangoproject.com/en/2.0/howto/static-files/ +SCRIPT_NAME = os.getenv('SCRIPT_NAME', '') # path for django_js_reverse to generate the javascript file containing all urls. Only done because the default command (collectstatic_js_reverse) fails to update the manifest JS_REVERSE_OUTPUT_PATH = os.path.join(BASE_DIR, "cookbook/static/django_js_reverse") - -JS_REVERSE_SCRIPT_PREFIX = os.getenv('JS_REVERSE_SCRIPT_PREFIX', os.getenv('SCRIPT_NAME', '')) +JS_REVERSE_SCRIPT_PREFIX = os.getenv('JS_REVERSE_SCRIPT_PREFIX', SCRIPT_NAME) STATIC_URL = os.getenv('STATIC_URL', '/static/') STATIC_ROOT = os.path.join(BASE_DIR, "staticfiles")