Merge pull request #1300 from TandoorRecipes/performance_refactor

Performance refactor

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")

cookbook/serializer.py

@@ -12,6 +12,7 @@
 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 @@
                              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 @@ def get_fields(self, *args, **kwargs):
             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 @@ def get_fields(self, *args, **kwargs):
         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 @@ def get_attribute(self, instance):
         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 = None
+        if request := self.context.get('request', None):
+            shared_users = getattr(request, '_shared_users', None)
+        if shared_users is None:
+            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 Meta:
 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()

cookbook/views/api.py

@@ -12,8 +12,9 @@
 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,13 +31,14 @@
 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,
                                                CustomIsShare, CustomIsShared, CustomIsUser,
                                                group_required)
 from cookbook.helper.recipe_html_import import get_recipe_from_source
-from cookbook.helper.recipe_search import get_facet, old_search, search_recipes
+from cookbook.helper.recipe_search import RecipeFacet, old_search, search_recipes
 from cookbook.helper.recipe_url_import import get_from_scraper
 from cookbook.helper.shopping_helper import list_from_recipe, shopping_helper
 from cookbook.models import (Automation, BookmarkletImport, CookLog, Food, FoodInheritField,
@@ -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 @@ def get_queryset(self):
             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<target>[^/.]+)', methods=['PUT'], )
-    @decorators.renderer_classes((TemplateHTMLRenderer, JSONRenderer))
+    @ decorators.action(detail=True, url_path='merge/(?P<target>[^/.]+)', 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 @@ def merge(self, request, pk, target):
                 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 @@ def get_queryset(self):
                 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')
+            return self.annotate_recipe(queryset=super().get_queryset(), request=self.request, serializer=self.serializer_class, tree=True)
+        self.queryset = self.queryset.filter(space=self.request.space).order_by('name')
 
-    @decorators.action(detail=True, url_path='move/(?P<parent>[^/.]+)', 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<parent>[^/.]+)', 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:
@@ -561,7 +604,7 @@ class RecipePagination(PageNumberPagination):
     max_page_size = 100
 
     def paginate_queryset(self, queryset, request, view=None):
-        self.facets = get_facet(qs=queryset, request=request)
+        self.facets = RecipeFacet(request, queryset=queryset)
         return super().paginate_queryset(queryset, request, view)
 
     def get_paginated_response(self, data):
@@ -570,7 +613,7 @@ def get_paginated_response(self, data):
             ('next', self.get_next_link()),
             ('previous', self.get_previous_link()),
             ('results', data),
-            ('facets', self.facets)
+            ('facets', self.facets.get_facets())
         ]))
 
 
@@ -608,8 +651,7 @@ def get_queryset(self):
             self.queryset = self.queryset.filter(space=self.request.space)
 
         self.queryset = search_recipes(self.request, self.queryset, self.request.GET)
-
-        return super().get_queryset()
+        return super().get_queryset().prefetch_related('cooklog_set')
 
     def list(self, request, *args, **kwargs):
         if self.request.GET.get('debug', False):
@@ -1089,10 +1131,20 @@ def ingredient_from_string(request):
 @group_required('user')
 def get_facets(request):
     key = request.GET.get('hash', None)
+    food = request.GET.get('food', None)
+    keyword = request.GET.get('keyword', None)
+    facets = RecipeFacet(request, hash_key=key)
+
+    if food:
+        results = facets.add_food_children(food)
+    elif keyword:
+        results = facets.add_keyword_children(keyword)
+    else:
+        results = facets.get_facets()
 
     return JsonResponse(
         {
-            'facets': get_facet(request=request, use_cache=False, hash_key=key),
+            'facets': results,
         },
         status=200
     )

recipes/middleware.py

@@ -1,3 +1,4 @@
+import time
 from os import getenv
 
 from django.conf import settings
@@ -13,19 +14,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]

recipes/settings.py

@@ -371,10 +371,10 @@
 # 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")