From 34ed38572755011cce5af6f36ffc31140fca0fae Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 25 Apr 2023 21:38:30 +0200 Subject: [PATCH 01/27] Add hook for random sorting --- mealie/repos/repository_generic.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mealie/repos/repository_generic.py b/mealie/repos/repository_generic.py index e96a1f11d54..9254d555094 100644 --- a/mealie/repos/repository_generic.py +++ b/mealie/repos/repository_generic.py @@ -378,4 +378,8 @@ def add_pagination_to_query(self, query: Select, pagination: PaginationQuery) -> query = query.order_by(order_attr) + elif pagination.order_by == "random": + order_attr = func.random() + query = query.order_by(order_attr) + return query.limit(pagination.per_page).offset((pagination.page - 1) * pagination.per_page), count, total_pages From 0a801c43947b2a27ed1f6999a6c42fc1ca573362 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 25 Apr 2023 21:38:45 +0200 Subject: [PATCH 02/27] Add random sorting to front page --- frontend/pages/index.vue | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frontend/pages/index.vue b/frontend/pages/index.vue index 56852cda944..781bd32f060 100644 --- a/frontend/pages/index.vue +++ b/frontend/pages/index.vue @@ -303,6 +303,11 @@ export default defineComponent({ name: i18n.tc("general.updated"), value: "update_at", }, + { + icon: $globals.icons.diceMultiple, + name: i18n.tc("general.random"), + value: "random", + }, ]; onMounted(() => { From 56643c260d16684747ff356215141c9c4e3be399 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 25 Apr 2023 23:12:57 +0200 Subject: [PATCH 03/27] Add multiple tests for random sorting. --- .../test_recipe_repository.py | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/tests/unit_tests/repository_tests/test_recipe_repository.py b/tests/unit_tests/repository_tests/test_recipe_repository.py index bc424f4eaa9..c867386b6db 100644 --- a/tests/unit_tests/repository_tests/test_recipe_repository.py +++ b/tests/unit_tests/repository_tests/test_recipe_repository.py @@ -200,6 +200,13 @@ def test_recipe_repo_pagination_by_categories(database: AllRepositories, unique_ for category in created_categories: assert category.id in category_ids + # Test random ordering with category filter + pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + random_ordered = [] + for i in range(5): + random_ordered.append(database.recipes.page_all(pagination_query, categories=[category_slug]).items) + assert not all(i == random_ordered[0] for i in random_ordered) + def test_recipe_repo_pagination_by_tags(database: AllRepositories, unique_user: TestUser): slug1, slug2 = (random_string(10) for _ in range(2)) @@ -279,6 +286,13 @@ def test_recipe_repo_pagination_by_tags(database: AllRepositories, unique_user: for tag in created_tags: assert tag.id in tag_ids + # Test random ordering with tag filter + pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + random_ordered = [] + for i in range(5): + random_ordered.append(database.recipes.page_all(pagination_query, tags=[tag_slug]).items) + assert not all(i == random_ordered[0] for i in random_ordered) + def test_recipe_repo_pagination_by_tools(database: AllRepositories, unique_user: TestUser): slug1, slug2 = (random_string(10) for _ in range(2)) @@ -360,6 +374,13 @@ def test_recipe_repo_pagination_by_tools(database: AllRepositories, unique_user: for tool in created_tools: assert tool.id in tool_ids + # Test random ordering with tools filter + pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + random_ordered = [] + for i in range(5): + random_ordered.append(database.recipes.page_all(pagination_query, tools=[tool_id]).items) + assert not all(i == random_ordered[0] for i in random_ordered) + def test_recipe_repo_pagination_by_foods(database: AllRepositories, unique_user: TestUser): slug1, slug2 = (random_string(10) for _ in range(2)) @@ -506,3 +527,10 @@ def test_recipe_repo_search(database: AllRepositories, unique_user: TestUser): print([r.name for r in normalized_result]) assert len(normalized_result) == 1 assert normalized_result[0].name == "Rátàtôuile" + + # Test random ordering with search + pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + random_ordered = [] + for i in range(5): + random_ordered.append(database.recipes.page_all(pagination_query, search="soup").items) + assert not all(i == random_ordered[0] for i in random_ordered) From 80f476a540d31aed9a7d9294d492ed25c423dfee Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 25 Apr 2023 23:20:50 +0200 Subject: [PATCH 04/27] Be extra sure that all recipes are returned. --- tests/unit_tests/repository_tests/test_recipe_repository.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/unit_tests/repository_tests/test_recipe_repository.py b/tests/unit_tests/repository_tests/test_recipe_repository.py index c867386b6db..2e728972f13 100644 --- a/tests/unit_tests/repository_tests/test_recipe_repository.py +++ b/tests/unit_tests/repository_tests/test_recipe_repository.py @@ -291,6 +291,7 @@ def test_recipe_repo_pagination_by_tags(database: AllRepositories, unique_user: random_ordered = [] for i in range(5): random_ordered.append(database.recipes.page_all(pagination_query, tags=[tag_slug]).items) + assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) @@ -379,6 +380,7 @@ def test_recipe_repo_pagination_by_tools(database: AllRepositories, unique_user: random_ordered = [] for i in range(5): random_ordered.append(database.recipes.page_all(pagination_query, tools=[tool_id]).items) + assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) From 5608f345d8aaea0c16ac65067ade185f887697d3 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 9 May 2023 08:27:57 +0900 Subject: [PATCH 05/27] Too stable random. seed doesn't reach backend. --- frontend/lib/api/user/recipes/recipe.ts | 2 ++ frontend/pages/index.vue | 3 ++- mealie/repos/repository_generic.py | 16 +++++++++++++--- mealie/routes/recipe/recipe_crud_routes.py | 6 ++++++ mealie/schema/response/pagination.py | 3 +++ 5 files changed, 26 insertions(+), 4 deletions(-) diff --git a/frontend/lib/api/user/recipes/recipe.ts b/frontend/lib/api/user/recipes/recipe.ts index 5499fd98b65..baa8fa75c31 100644 --- a/frontend/lib/api/user/recipes/recipe.ts +++ b/frontend/lib/api/user/recipes/recipe.ts @@ -78,6 +78,8 @@ export type RecipeSearchQuery = { page?: number; perPage?: number; orderBy?: string; + + timestamp: number; // obligatory for now }; export class RecipeAPI extends BaseCRUDAPI { diff --git a/frontend/pages/index.vue b/frontend/pages/index.vue index 781bd32f060..fe4445b6f05 100644 --- a/frontend/pages/index.vue +++ b/frontend/pages/index.vue @@ -212,7 +212,7 @@ export default defineComponent({ foods: toIDArray(selectedFoods.value), tags: toIDArray(selectedTags.value), tools: toIDArray(selectedTools.value), - + timestamp: Date.now().toString(), // shows up in address bar, for debugging // Only add the query param if it's or not default ...{ auto: state.value.auto ? undefined : "false", @@ -239,6 +239,7 @@ export default defineComponent({ requireAllFoods: state.value.requireAllFoods, orderBy: state.value.orderBy, orderDirection: state.value.orderDirection, + timestamp: Date.now(), // never arrives at backend! }; } diff --git a/mealie/repos/repository_generic.py b/mealie/repos/repository_generic.py index 9254d555094..cbddbf9bf4b 100644 --- a/mealie/repos/repository_generic.py +++ b/mealie/repos/repository_generic.py @@ -2,11 +2,12 @@ from collections.abc import Iterable from math import ceil +import random from typing import Any, Generic, TypeVar from fastapi import HTTPException from pydantic import UUID4, BaseModel -from sqlalchemy import Select, delete, func, select +from sqlalchemy import case, Select, delete, func, select from sqlalchemy.orm.session import Session from sqlalchemy.sql import sqltypes @@ -379,7 +380,16 @@ def add_pagination_to_query(self, query: Select, pagination: PaginationQuery) -> query = query.order_by(order_attr) elif pagination.order_by == "random": - order_attr = func.random() - query = query.order_by(order_attr) + # randomize outside of database, since not all db's can set random seeds + # this solution is db-independent & stable to paging + temp_query = query.with_only_columns(self.model.id) + allids = self.session.execute(temp_query).scalars().all() # fast because id is indexed + order = list(range(len(allids))) + self.logger.warning(f"seed {int(pagination.timestamp)}") + random.seed(int(pagination.timestamp)) + random.shuffle(order) + random_dict = dict(zip(allids, order)) + case_stmt = case(random_dict, value=self.model.id) + query = query.order_by(case_stmt) return query.limit(pagination.per_page).offset((pagination.page - 1) * pagination.per_page), count, total_pages diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index ce3538b7f54..4f3af573afc 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -1,4 +1,6 @@ +from datetime import datetime from functools import cached_property +import logging from shutil import copyfileobj from zipfile import ZipFile @@ -252,6 +254,10 @@ def get_all( if search_query.cookbook is None: raise HTTPException(status_code=404, detail="cookbook not found") + log = logging.getLogger("recipe_crud_routes") + q.timestamp = search_query.timestamp # propagate time of search to enable stable randomization across pages + log.warning(f"search {search_query.timestamp}") + log.warning(f"pagination {q.timestamp}") pagination_response = self.repo.page_all( pagination=q, cookbook=cookbook_data, diff --git a/mealie/schema/response/pagination.py b/mealie/schema/response/pagination.py index 11b23c3fa96..a718df6d77b 100644 --- a/mealie/schema/response/pagination.py +++ b/mealie/schema/response/pagination.py @@ -1,3 +1,4 @@ +from datetime import datetime import enum from typing import Any, Generic, TypeVar from urllib.parse import parse_qs, urlencode, urlsplit, urlunsplit @@ -23,6 +24,7 @@ class RecipeSearchQuery(MealieModel): require_all_tools: bool = False require_all_foods: bool = False search: str | None + timestamp: float = datetime.now().timestamp() # never set by frontend. currently set for debug class PaginationQuery(MealieModel): @@ -31,6 +33,7 @@ class PaginationQuery(MealieModel): order_by: str = "created_at" order_direction: OrderDirection = OrderDirection.desc query_filter: str | None = None + timestamp: float = datetime.now().timestamp() # never set by frontend. currently set for debug class PaginationBase(GenericModel, Generic[DataT]): From ef6fc0f74cd4863e308f56ee6346f36e430e079c Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 9 May 2023 09:35:30 +0900 Subject: [PATCH 06/27] add timestamp to useRecipeSearch --- frontend/composables/recipes/use-recipe-search.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/composables/recipes/use-recipe-search.ts b/frontend/composables/recipes/use-recipe-search.ts index c18f27328b0..7dca8be1948 100644 --- a/frontend/composables/recipes/use-recipe-search.ts +++ b/frontend/composables/recipes/use-recipe-search.ts @@ -31,6 +31,7 @@ export function useRecipeSearch(api: UserApi): UseRecipeSearchReturn { orderBy: "name", orderDirection: "asc", perPage: 20, + timestamp: Date.now(), }); if (error) { From aa548eca45a29172ab42ee1102a90a57044f4376 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 9 May 2023 09:55:58 +0900 Subject: [PATCH 07/27] Update randomization tests for timestamp seeding --- .../repository_tests/test_recipe_repository.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/unit_tests/repository_tests/test_recipe_repository.py b/tests/unit_tests/repository_tests/test_recipe_repository.py index 2e728972f13..32560d9302a 100644 --- a/tests/unit_tests/repository_tests/test_recipe_repository.py +++ b/tests/unit_tests/repository_tests/test_recipe_repository.py @@ -1,3 +1,4 @@ +from datetime import datetime from typing import cast from mealie.repos.repository_factory import AllRepositories @@ -204,6 +205,7 @@ def test_recipe_repo_pagination_by_categories(database: AllRepositories, unique_ pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) random_ordered = [] for i in range(5): + pagination_query.timestamp = datetime.now().timestamp() random_ordered.append(database.recipes.page_all(pagination_query, categories=[category_slug]).items) assert not all(i == random_ordered[0] for i in random_ordered) @@ -290,6 +292,7 @@ def test_recipe_repo_pagination_by_tags(database: AllRepositories, unique_user: pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) random_ordered = [] for i in range(5): + pagination_query.timestamp = datetime.now().timestamp() random_ordered.append(database.recipes.page_all(pagination_query, tags=[tag_slug]).items) assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) @@ -379,6 +382,7 @@ def test_recipe_repo_pagination_by_tools(database: AllRepositories, unique_user: pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) random_ordered = [] for i in range(5): + pagination_query.timestamp = datetime.now().timestamp() random_ordered.append(database.recipes.page_all(pagination_query, tools=[tool_id]).items) assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) @@ -453,6 +457,14 @@ def test_recipe_repo_pagination_by_foods(database: AllRepositories, unique_user: assert len(recipes_with_either_food) == 20 + pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + random_ordered = [] + for i in range(5): + pagination_query.timestamp = datetime.now().timestamp() + random_ordered.append(database.recipes.page_all(pagination_query, foods=[food_id]).items) + assert len(random_ordered[0]) == 15 + assert not all(i == random_ordered[0] for i in random_ordered) + def test_recipe_repo_search(database: AllRepositories, unique_user: TestUser): ingredient_1 = random_string(10) @@ -534,5 +546,6 @@ def test_recipe_repo_search(database: AllRepositories, unique_user: TestUser): pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) random_ordered = [] for i in range(5): + pagination_query.timestamp = datetime.now().timestamp() random_ordered.append(database.recipes.page_all(pagination_query, search="soup").items) assert not all(i == random_ordered[0] for i in random_ordered) From c265521a7b6ca2ac5202bbcf085a4bce50847046 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 9 May 2023 10:12:28 +0900 Subject: [PATCH 08/27] ruff cleanup --- mealie/repos/repository_generic.py | 10 +++++----- mealie/routes/recipe/recipe_crud_routes.py | 3 +-- mealie/schema/response/pagination.py | 2 +- 3 files changed, 7 insertions(+), 8 deletions(-) diff --git a/mealie/repos/repository_generic.py b/mealie/repos/repository_generic.py index cbddbf9bf4b..761f96585a2 100644 --- a/mealie/repos/repository_generic.py +++ b/mealie/repos/repository_generic.py @@ -1,13 +1,13 @@ from __future__ import annotations +import random from collections.abc import Iterable from math import ceil -import random from typing import Any, Generic, TypeVar from fastapi import HTTPException from pydantic import UUID4, BaseModel -from sqlalchemy import case, Select, delete, func, select +from sqlalchemy import Select, case, delete, func, select from sqlalchemy.orm.session import Session from sqlalchemy.sql import sqltypes @@ -385,10 +385,10 @@ def add_pagination_to_query(self, query: Select, pagination: PaginationQuery) -> temp_query = query.with_only_columns(self.model.id) allids = self.session.execute(temp_query).scalars().all() # fast because id is indexed order = list(range(len(allids))) - self.logger.warning(f"seed {int(pagination.timestamp)}") - random.seed(int(pagination.timestamp)) + self.logger.warning(f"seed {pagination.timestamp}") + random.seed(pagination.timestamp) random.shuffle(order) - random_dict = dict(zip(allids, order)) + random_dict = dict(zip(allids, order, strict=True)) case_stmt = case(random_dict, value=self.model.id) query = query.order_by(case_stmt) diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index 4f3af573afc..4d140e226e4 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -1,6 +1,5 @@ -from datetime import datetime -from functools import cached_property import logging +from functools import cached_property from shutil import copyfileobj from zipfile import ZipFile diff --git a/mealie/schema/response/pagination.py b/mealie/schema/response/pagination.py index a718df6d77b..60a1da341e0 100644 --- a/mealie/schema/response/pagination.py +++ b/mealie/schema/response/pagination.py @@ -1,5 +1,5 @@ -from datetime import datetime import enum +from datetime import datetime from typing import Any, Generic, TypeVar from urllib.parse import parse_qs, urlencode, urlsplit, urlunsplit From 04ddf00f249dc2816ca5ebe4c3fcc0eb098ac110 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Wed, 10 May 2023 16:36:06 +0900 Subject: [PATCH 09/27] pass timestamp separately in getAll --- frontend/composables/recipes/use-recipes.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frontend/composables/recipes/use-recipes.ts b/frontend/composables/recipes/use-recipes.ts index b353a55ef8d..dfe6665ba04 100644 --- a/frontend/composables/recipes/use-recipes.ts +++ b/frontend/composables/recipes/use-recipes.ts @@ -1,8 +1,8 @@ import { useAsync, ref } from "@nuxtjs/composition-api"; import { useAsyncKey } from "../use-utils"; import { useUserApi } from "~/composables/api"; -import {Recipe} from "~/lib/api/types/recipe"; -import {RecipeSearchQuery} from "~/lib/api/user/recipes/recipe"; +import { Recipe } from "~/lib/api/types/recipe"; +import { RecipeSearchQuery } from "~/lib/api/user/recipes/recipe"; export const allRecipes = ref([]); export const recentRecipes = ref([]); @@ -33,6 +33,7 @@ export const useLazyRecipes = function () { requireAllTools: query?.requireAllTools, foods: query?.foods, requireAllFoods: query?.requireAllFoods, + timestamp: query?.timestamp, queryFilter, }); return data ? data.items : []; From 83f6e71a0ce531242cbc3d3e4f8d19fc3136e7af Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Wed, 10 May 2023 16:36:18 +0900 Subject: [PATCH 10/27] remove debugging log items --- mealie/repos/repository_generic.py | 1 - mealie/routes/recipe/recipe_crud_routes.py | 4 ---- 2 files changed, 5 deletions(-) diff --git a/mealie/repos/repository_generic.py b/mealie/repos/repository_generic.py index 761f96585a2..30841c402b8 100644 --- a/mealie/repos/repository_generic.py +++ b/mealie/repos/repository_generic.py @@ -385,7 +385,6 @@ def add_pagination_to_query(self, query: Select, pagination: PaginationQuery) -> temp_query = query.with_only_columns(self.model.id) allids = self.session.execute(temp_query).scalars().all() # fast because id is indexed order = list(range(len(allids))) - self.logger.warning(f"seed {pagination.timestamp}") random.seed(pagination.timestamp) random.shuffle(order) random_dict = dict(zip(allids, order, strict=True)) diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index 4d140e226e4..a2e50978036 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -1,4 +1,3 @@ -import logging from functools import cached_property from shutil import copyfileobj from zipfile import ZipFile @@ -253,10 +252,7 @@ def get_all( if search_query.cookbook is None: raise HTTPException(status_code=404, detail="cookbook not found") - log = logging.getLogger("recipe_crud_routes") q.timestamp = search_query.timestamp # propagate time of search to enable stable randomization across pages - log.warning(f"search {search_query.timestamp}") - log.warning(f"pagination {q.timestamp}") pagination_response = self.repo.page_all( pagination=q, cookbook=cookbook_data, From 16b795525384fa5478488d39d2f28d57a28d1c84 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Thu, 11 May 2023 07:31:06 +0900 Subject: [PATCH 11/27] remove timestamp from address bar --- frontend/pages/index.vue | 1 - 1 file changed, 1 deletion(-) diff --git a/frontend/pages/index.vue b/frontend/pages/index.vue index fe4445b6f05..b12826c6745 100644 --- a/frontend/pages/index.vue +++ b/frontend/pages/index.vue @@ -212,7 +212,6 @@ export default defineComponent({ foods: toIDArray(selectedFoods.value), tags: toIDArray(selectedTags.value), tools: toIDArray(selectedTools.value), - timestamp: Date.now().toString(), // shows up in address bar, for debugging // Only add the query param if it's or not default ...{ auto: state.value.auto ? undefined : "false", From d37765adb426cc15475f31b0b328608c516fd8fb Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Thu, 11 May 2023 07:31:20 +0900 Subject: [PATCH 12/27] remove defaults from backend timestamps --- mealie/schema/response/pagination.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/mealie/schema/response/pagination.py b/mealie/schema/response/pagination.py index 60a1da341e0..a00645ce2e7 100644 --- a/mealie/schema/response/pagination.py +++ b/mealie/schema/response/pagination.py @@ -24,7 +24,7 @@ class RecipeSearchQuery(MealieModel): require_all_tools: bool = False require_all_foods: bool = False search: str | None - timestamp: float = datetime.now().timestamp() # never set by frontend. currently set for debug + timestamp: float class PaginationQuery(MealieModel): @@ -33,7 +33,7 @@ class PaginationQuery(MealieModel): order_by: str = "created_at" order_direction: OrderDirection = OrderDirection.desc query_filter: str | None = None - timestamp: float = datetime.now().timestamp() # never set by frontend. currently set for debug + timestamp: float # never set by frontend. currently set for debug class PaginationBase(GenericModel, Generic[DataT]): From f4056c4bece19d427f01d5fdbc27afd3b5367a20 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Thu, 11 May 2023 09:01:05 +0900 Subject: [PATCH 13/27] timestamp should be optional --- mealie/schema/response/pagination.py | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/mealie/schema/response/pagination.py b/mealie/schema/response/pagination.py index a00645ce2e7..b1ae2401205 100644 --- a/mealie/schema/response/pagination.py +++ b/mealie/schema/response/pagination.py @@ -1,5 +1,4 @@ import enum -from datetime import datetime from typing import Any, Generic, TypeVar from urllib.parse import parse_qs, urlencode, urlsplit, urlunsplit @@ -24,7 +23,7 @@ class RecipeSearchQuery(MealieModel): require_all_tools: bool = False require_all_foods: bool = False search: str | None - timestamp: float + timestamp: float | None class PaginationQuery(MealieModel): @@ -33,7 +32,7 @@ class PaginationQuery(MealieModel): order_by: str = "created_at" order_direction: OrderDirection = OrderDirection.desc query_filter: str | None = None - timestamp: float # never set by frontend. currently set for debug + timestamp: float | None class PaginationBase(GenericModel, Generic[DataT]): From 112b891af8f836b19436ddff9108650b00ed8bbb Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Thu, 11 May 2023 09:23:05 +0900 Subject: [PATCH 14/27] fix edge case: query without timestamp --- mealie/routes/recipe/recipe_crud_routes.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index a2e50978036..c1743112619 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -1,3 +1,4 @@ +from datetime import datetime from functools import cached_property from shutil import copyfileobj from zipfile import ZipFile @@ -252,7 +253,10 @@ def get_all( if search_query.cookbook is None: raise HTTPException(status_code=404, detail="cookbook not found") + if not search_query.timestamp: + search_query.timestamp = datetime.now().timestamp() q.timestamp = search_query.timestamp # propagate time of search to enable stable randomization across pages + pagination_response = self.repo.page_all( pagination=q, cookbook=cookbook_data, From 92c94ae9eda45bc4bdce5ed1bc960446d0dbb8f8 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Thu, 11 May 2023 09:27:30 +0900 Subject: [PATCH 15/27] similar edge case: no timestamp in pagination --- mealie/repos/repository_generic.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/mealie/repos/repository_generic.py b/mealie/repos/repository_generic.py index 30841c402b8..7e1d9741b16 100644 --- a/mealie/repos/repository_generic.py +++ b/mealie/repos/repository_generic.py @@ -1,5 +1,6 @@ from __future__ import annotations +from datetime import datetime import random from collections.abc import Iterable from math import ceil @@ -385,6 +386,9 @@ def add_pagination_to_query(self, query: Select, pagination: PaginationQuery) -> temp_query = query.with_only_columns(self.model.id) allids = self.session.execute(temp_query).scalars().all() # fast because id is indexed order = list(range(len(allids))) + if not pagination.timestamp: + pagination.timestamp = datetime.now().timestamp() + random.seed(pagination.timestamp) random.shuffle(order) random_dict = dict(zip(allids, order, strict=True)) From 0bdfd6bab0077dbe313ed385e4cafde30356181b Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Thu, 11 May 2023 09:31:26 +0900 Subject: [PATCH 16/27] ruff :/ --- mealie/repos/repository_generic.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mealie/repos/repository_generic.py b/mealie/repos/repository_generic.py index 7e1d9741b16..356646a88f8 100644 --- a/mealie/repos/repository_generic.py +++ b/mealie/repos/repository_generic.py @@ -1,8 +1,8 @@ from __future__ import annotations -from datetime import datetime import random from collections.abc import Iterable +from datetime import datetime from math import ceil from typing import Any, Generic, TypeVar From 8b4f6de580657d2a60005fe0e4010b3169c026d7 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Thu, 11 May 2023 10:19:13 +0900 Subject: [PATCH 17/27] better edge case handling --- mealie/repos/repository_generic.py | 4 ---- mealie/routes/recipe/recipe_crud_routes.py | 7 ++++--- 2 files changed, 4 insertions(+), 7 deletions(-) diff --git a/mealie/repos/repository_generic.py b/mealie/repos/repository_generic.py index 356646a88f8..30841c402b8 100644 --- a/mealie/repos/repository_generic.py +++ b/mealie/repos/repository_generic.py @@ -2,7 +2,6 @@ import random from collections.abc import Iterable -from datetime import datetime from math import ceil from typing import Any, Generic, TypeVar @@ -386,9 +385,6 @@ def add_pagination_to_query(self, query: Select, pagination: PaginationQuery) -> temp_query = query.with_only_columns(self.model.id) allids = self.session.execute(temp_query).scalars().all() # fast because id is indexed order = list(range(len(allids))) - if not pagination.timestamp: - pagination.timestamp = datetime.now().timestamp() - random.seed(pagination.timestamp) random.shuffle(order) random_dict = dict(zip(allids, order, strict=True)) diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index c1743112619..ceb649f0bcd 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -253,9 +253,10 @@ def get_all( if search_query.cookbook is None: raise HTTPException(status_code=404, detail="cookbook not found") - if not search_query.timestamp: - search_query.timestamp = datetime.now().timestamp() - q.timestamp = search_query.timestamp # propagate time of search to enable stable randomization across pages + if q.order_by == "random": + if not search_query.timestamp: + search_query.timestamp = datetime.now().timestamp() + q.timestamp = search_query.timestamp # propagate time of search to enable stable randomization across pages pagination_response = self.repo.page_all( pagination=q, From 9fe47d7880304d25bed749748e60d1db26512dca Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Thu, 11 May 2023 19:10:49 +0200 Subject: [PATCH 18/27] stabilize random search test w/more recipes --- .../test_recipe_repository.py | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/unit_tests/repository_tests/test_recipe_repository.py b/tests/unit_tests/repository_tests/test_recipe_repository.py index 32560d9302a..e906c0a392e 100644 --- a/tests/unit_tests/repository_tests/test_recipe_repository.py +++ b/tests/unit_tests/repository_tests/test_recipe_repository.py @@ -504,6 +504,37 @@ def test_recipe_repo_search(database: AllRepositories, unique_user: TestUser): group_id=unique_user.group_id, name="Rátàtôuile", ), + # Add a bunch of recipes for stable randomization + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=f"{random_string(10)} soup", + ), + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=f"{random_string(10)} soup", + ), + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=f"{random_string(10)} soup", + ), + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=f"{random_string(10)} soup", + ), + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=f"{random_string(10)} soup", + ), + Recipe( + user_id=unique_user.user_id, + group_id=unique_user.group_id, + name=f"{random_string(10)} soup", + ), ] for recipe in recipes: From ab46b880fe4060a693eff72abe986d94a4bbe9ca Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 16 May 2023 10:01:59 +0200 Subject: [PATCH 19/27] better pagination seeding --- frontend/composables/recipes/use-recipe-search.ts | 2 +- frontend/composables/recipes/use-recipes.ts | 3 ++- frontend/lib/api/user/recipes/recipe.ts | 2 +- frontend/pages/index.vue | 2 +- mealie/repos/repository_generic.py | 2 +- mealie/routes/recipe/recipe_crud_routes.py | 11 ++++++----- mealie/schema/response/pagination.py | 12 +++++++++--- 7 files changed, 21 insertions(+), 13 deletions(-) diff --git a/frontend/composables/recipes/use-recipe-search.ts b/frontend/composables/recipes/use-recipe-search.ts index 7dca8be1948..cb27062bead 100644 --- a/frontend/composables/recipes/use-recipe-search.ts +++ b/frontend/composables/recipes/use-recipe-search.ts @@ -31,7 +31,7 @@ export function useRecipeSearch(api: UserApi): UseRecipeSearchReturn { orderBy: "name", orderDirection: "asc", perPage: 20, - timestamp: Date.now(), + _searchSeed: Date.now().toString(), }); if (error) { diff --git a/frontend/composables/recipes/use-recipes.ts b/frontend/composables/recipes/use-recipes.ts index dfe6665ba04..1c721e3f228 100644 --- a/frontend/composables/recipes/use-recipes.ts +++ b/frontend/composables/recipes/use-recipes.ts @@ -23,6 +23,8 @@ export const useLazyRecipes = function () { const { data } = await api.recipes.getAll(page, perPage, { orderBy, orderDirection, + paginationSeed: query?._searchSeed, // propagate searchSeed to stabilize random order pagination + searchSeed: query?._searchSeed, // unused, but pass it along for completeness of data search: query?.search, cookbook: query?.cookbook, categories: query?.categories, @@ -33,7 +35,6 @@ export const useLazyRecipes = function () { requireAllTools: query?.requireAllTools, foods: query?.foods, requireAllFoods: query?.requireAllFoods, - timestamp: query?.timestamp, queryFilter, }); return data ? data.items : []; diff --git a/frontend/lib/api/user/recipes/recipe.ts b/frontend/lib/api/user/recipes/recipe.ts index baa8fa75c31..879ffecdc37 100644 --- a/frontend/lib/api/user/recipes/recipe.ts +++ b/frontend/lib/api/user/recipes/recipe.ts @@ -79,7 +79,7 @@ export type RecipeSearchQuery = { perPage?: number; orderBy?: string; - timestamp: number; // obligatory for now + _searchSeed?: string; }; export class RecipeAPI extends BaseCRUDAPI { diff --git a/frontend/pages/index.vue b/frontend/pages/index.vue index b12826c6745..a306e852464 100644 --- a/frontend/pages/index.vue +++ b/frontend/pages/index.vue @@ -238,7 +238,7 @@ export default defineComponent({ requireAllFoods: state.value.requireAllFoods, orderBy: state.value.orderBy, orderDirection: state.value.orderDirection, - timestamp: Date.now(), // never arrives at backend! + _searchSeed: Date.now().toString() }; } diff --git a/mealie/repos/repository_generic.py b/mealie/repos/repository_generic.py index 30841c402b8..76abb55b7b5 100644 --- a/mealie/repos/repository_generic.py +++ b/mealie/repos/repository_generic.py @@ -385,7 +385,7 @@ def add_pagination_to_query(self, query: Select, pagination: PaginationQuery) -> temp_query = query.with_only_columns(self.model.id) allids = self.session.execute(temp_query).scalars().all() # fast because id is indexed order = list(range(len(allids))) - random.seed(pagination.timestamp) + random.seed(pagination.pagination_seed) random.shuffle(order) random_dict = dict(zip(allids, order, strict=True)) case_stmt = case(random_dict, value=self.model.id) diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index ceb649f0bcd..80a39667242 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -1,4 +1,3 @@ -from datetime import datetime from functools import cached_property from shutil import copyfileobj from zipfile import ZipFile @@ -253,10 +252,12 @@ def get_all( if search_query.cookbook is None: raise HTTPException(status_code=404, detail="cookbook not found") - if q.order_by == "random": - if not search_query.timestamp: - search_query.timestamp = datetime.now().timestamp() - q.timestamp = search_query.timestamp # propagate time of search to enable stable randomization across pages + if ( + q.order_by == "random" and not q.pagination_seed + ): # this should be already taken care of upstream, but just in case + q.pagination_seed = ( + search_query._search_seed + ) # propagate time of search -> stable randomization across pages pagination_response = self.repo.page_all( pagination=q, diff --git a/mealie/schema/response/pagination.py b/mealie/schema/response/pagination.py index b1ae2401205..5863f88d64e 100644 --- a/mealie/schema/response/pagination.py +++ b/mealie/schema/response/pagination.py @@ -3,7 +3,7 @@ from urllib.parse import parse_qs, urlencode, urlsplit, urlunsplit from humps import camelize -from pydantic import UUID4, BaseModel +from pydantic import UUID4, BaseModel, validator from pydantic.generics import GenericModel from mealie.schema._mealie import MealieModel @@ -23,7 +23,7 @@ class RecipeSearchQuery(MealieModel): require_all_tools: bool = False require_all_foods: bool = False search: str | None - timestamp: float | None + _search_seed: str | None = None class PaginationQuery(MealieModel): @@ -32,7 +32,13 @@ class PaginationQuery(MealieModel): order_by: str = "created_at" order_direction: OrderDirection = OrderDirection.desc query_filter: str | None = None - timestamp: float | None + pagination_seed: str | None = None + + @validator("pagination_seed", always=True, pre=True) + def validate_randseed(cls, pagination_seed, values): + if values.get("order_by") == "random" and not pagination_seed: + raise ValueError("pagination_seed required for when orderBy is random") + return pagination_seed class PaginationBase(GenericModel, Generic[DataT]): From 14df0b70213f7f5523b7b1eb1cba9ab142f479f6 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 16 May 2023 10:02:10 +0200 Subject: [PATCH 20/27] update pagination seed test --- .../test_recipe_repository.py | 42 +++++++++++++++---- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/tests/unit_tests/repository_tests/test_recipe_repository.py b/tests/unit_tests/repository_tests/test_recipe_repository.py index e906c0a392e..e950d5bd7c7 100644 --- a/tests/unit_tests/repository_tests/test_recipe_repository.py +++ b/tests/unit_tests/repository_tests/test_recipe_repository.py @@ -202,10 +202,16 @@ def test_recipe_repo_pagination_by_categories(database: AllRepositories, unique_ assert category.id in category_ids # Test random ordering with category filter - pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + pagination_query = PaginationQuery( + page=1, + per_page=-1, + order_by="random", + pagination_seed=datetime.now().timestamp(), + order_direction=OrderDirection.asc, + ) random_ordered = [] for i in range(5): - pagination_query.timestamp = datetime.now().timestamp() + pagination_query.pagination_seed = datetime.now().timestamp() random_ordered.append(database.recipes.page_all(pagination_query, categories=[category_slug]).items) assert not all(i == random_ordered[0] for i in random_ordered) @@ -289,10 +295,16 @@ def test_recipe_repo_pagination_by_tags(database: AllRepositories, unique_user: assert tag.id in tag_ids # Test random ordering with tag filter - pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + pagination_query = PaginationQuery( + page=1, + per_page=-1, + order_by="random", + pagination_seed=datetime.now().timestamp(), + order_direction=OrderDirection.asc, + ) random_ordered = [] for i in range(5): - pagination_query.timestamp = datetime.now().timestamp() + pagination_query.pagination_seed = datetime.now().timestamp() random_ordered.append(database.recipes.page_all(pagination_query, tags=[tag_slug]).items) assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) @@ -382,7 +394,7 @@ def test_recipe_repo_pagination_by_tools(database: AllRepositories, unique_user: pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) random_ordered = [] for i in range(5): - pagination_query.timestamp = datetime.now().timestamp() + pagination_query.pagination_seed = datetime.now().timestamp() random_ordered.append(database.recipes.page_all(pagination_query, tools=[tool_id]).items) assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) @@ -457,10 +469,16 @@ def test_recipe_repo_pagination_by_foods(database: AllRepositories, unique_user: assert len(recipes_with_either_food) == 20 - pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + pagination_query = PaginationQuery( + page=1, + per_page=-1, + order_by="random", + pagination_seed=datetime.now().timestamp(), + order_direction=OrderDirection.asc, + ) random_ordered = [] for i in range(5): - pagination_query.timestamp = datetime.now().timestamp() + pagination_query.pagination_seed = datetime.now().timestamp() random_ordered.append(database.recipes.page_all(pagination_query, foods=[food_id]).items) assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) @@ -574,9 +592,15 @@ def test_recipe_repo_search(database: AllRepositories, unique_user: TestUser): assert normalized_result[0].name == "Rátàtôuile" # Test random ordering with search - pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + pagination_query = PaginationQuery( + page=1, + per_page=-1, + order_by="random", + pagination_seed=datetime.now().timestamp(), + order_direction=OrderDirection.asc, + ) random_ordered = [] for i in range(5): - pagination_query.timestamp = datetime.now().timestamp() + pagination_query.pagination_seed = datetime.now().timestamp() random_ordered.append(database.recipes.page_all(pagination_query, search="soup").items) assert not all(i == random_ordered[0] for i in random_ordered) From 88facf83c68187667d7b1efea94d4b6765c013a3 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 16 May 2023 11:08:27 +0200 Subject: [PATCH 21/27] remove redundant random/seed check --- mealie/routes/recipe/recipe_crud_routes.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index 80a39667242..ce3538b7f54 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -252,13 +252,6 @@ def get_all( if search_query.cookbook is None: raise HTTPException(status_code=404, detail="cookbook not found") - if ( - q.order_by == "random" and not q.pagination_seed - ): # this should be already taken care of upstream, but just in case - q.pagination_seed = ( - search_query._search_seed - ) # propagate time of search -> stable randomization across pages - pagination_response = self.repo.page_all( pagination=q, cookbook=cookbook_data, From 71ac6e2acfb36faf669aa466be4079a878394b07 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Tue, 16 May 2023 12:20:27 +0200 Subject: [PATCH 22/27] Test for api routes to random sorting. --- .../user_recipe_tests/test_recipe_crud.py | 25 ++++++++++++++++++ .../test_recipe_repository.py | 26 ++++++++++++------- 2 files changed, 41 insertions(+), 10 deletions(-) diff --git a/tests/integration_tests/user_recipe_tests/test_recipe_crud.py b/tests/integration_tests/user_recipe_tests/test_recipe_crud.py index 305311ed89d..9ad1281aac4 100644 --- a/tests/integration_tests/user_recipe_tests/test_recipe_crud.py +++ b/tests/integration_tests/user_recipe_tests/test_recipe_crud.py @@ -424,3 +424,28 @@ def test_get_recipe_by_slug_or_id(api_client: TestClient, unique_user: utils.Tes recipe_data = response.json() assert recipe_data["slug"] == slug assert recipe_data["id"] == recipe_id + + +def test_get_random_order(api_client: TestClient, unique_user: utils.TestUser): + # Create more recipes for stable random ordering + slugs = [random_string(10) for _ in range(7)] + for slug in slugs: + response = api_client.post(api_routes.recipes, json={"name": slug}, headers=unique_user.token) + assert response.status_code == 201 + assert json.loads(response.text) == slug + + goodparams = {"page": 1, "perPage": -1, "orderBy": "random", "paginationSeed": "abcdefg"} + response = api_client.get(api_routes.recipes, params=goodparams, headers=unique_user.token) + assert response.status_code == 200 + + seed1_params = {"page": 1, "perPage": -1, "orderBy": "random", "paginationSeed": "abcdefg"} + seed2_params = {"page": 1, "perPage": -1, "orderBy": "random", "paginationSeed": "gfedcba"} + data1 = api_client.get(api_routes.recipes, params=seed1_params, headers=unique_user.token).json() + data2 = api_client.get(api_routes.recipes, params=seed2_params, headers=unique_user.token).json() + data1_new = api_client.get(api_routes.recipes, params=seed1_params, headers=unique_user.token).json() + assert data1["items"][0]["slug"] != data2["items"][0]["slug"] # new seed -> new order + assert data1["items"][0]["slug"] == data1_new["items"][0]["slug"] # same seed -> same order + + badparams = {"page": 1, "perPage": -1, "orderBy": "random"} + response = api_client.get(api_routes.recipes, params=badparams, headers=unique_user.token) + assert response.status_code == 422 diff --git a/tests/unit_tests/repository_tests/test_recipe_repository.py b/tests/unit_tests/repository_tests/test_recipe_repository.py index e950d5bd7c7..c336188c5ca 100644 --- a/tests/unit_tests/repository_tests/test_recipe_repository.py +++ b/tests/unit_tests/repository_tests/test_recipe_repository.py @@ -206,12 +206,12 @@ def test_recipe_repo_pagination_by_categories(database: AllRepositories, unique_ page=1, per_page=-1, order_by="random", - pagination_seed=datetime.now().timestamp(), + pagination_seed=str(datetime.now()), order_direction=OrderDirection.asc, ) random_ordered = [] for i in range(5): - pagination_query.pagination_seed = datetime.now().timestamp() + pagination_query.pagination_seed = str(datetime.now()) random_ordered.append(database.recipes.page_all(pagination_query, categories=[category_slug]).items) assert not all(i == random_ordered[0] for i in random_ordered) @@ -299,12 +299,12 @@ def test_recipe_repo_pagination_by_tags(database: AllRepositories, unique_user: page=1, per_page=-1, order_by="random", - pagination_seed=datetime.now().timestamp(), + pagination_seed=str(datetime.now()), order_direction=OrderDirection.asc, ) random_ordered = [] for i in range(5): - pagination_query.pagination_seed = datetime.now().timestamp() + pagination_query.pagination_seed = str(datetime.now()) random_ordered.append(database.recipes.page_all(pagination_query, tags=[tag_slug]).items) assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) @@ -391,10 +391,16 @@ def test_recipe_repo_pagination_by_tools(database: AllRepositories, unique_user: assert tool.id in tool_ids # Test random ordering with tools filter - pagination_query = PaginationQuery(page=1, per_page=-1, order_by="random", order_direction=OrderDirection.asc) + pagination_query = PaginationQuery( + page=1, + per_page=-1, + order_by="random", + pagination_seed=str(datetime.now()), + order_direction=OrderDirection.asc, + ) random_ordered = [] for i in range(5): - pagination_query.pagination_seed = datetime.now().timestamp() + pagination_query.pagination_seed = str(datetime.now()) random_ordered.append(database.recipes.page_all(pagination_query, tools=[tool_id]).items) assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) @@ -473,12 +479,12 @@ def test_recipe_repo_pagination_by_foods(database: AllRepositories, unique_user: page=1, per_page=-1, order_by="random", - pagination_seed=datetime.now().timestamp(), + pagination_seed=str(datetime.now()), order_direction=OrderDirection.asc, ) random_ordered = [] for i in range(5): - pagination_query.pagination_seed = datetime.now().timestamp() + pagination_query.pagination_seed = str(datetime.now()) random_ordered.append(database.recipes.page_all(pagination_query, foods=[food_id]).items) assert len(random_ordered[0]) == 15 assert not all(i == random_ordered[0] for i in random_ordered) @@ -596,11 +602,11 @@ def test_recipe_repo_search(database: AllRepositories, unique_user: TestUser): page=1, per_page=-1, order_by="random", - pagination_seed=datetime.now().timestamp(), + pagination_seed=str(datetime.now()), order_direction=OrderDirection.asc, ) random_ordered = [] for i in range(5): - pagination_query.pagination_seed = datetime.now().timestamp() + pagination_query.pagination_seed = str(datetime.now()) random_ordered.append(database.recipes.page_all(pagination_query, search="soup").items) assert not all(i == random_ordered[0] for i in random_ordered) From ff4b657aba9427c7b390221c7aaade7ea09bbe81 Mon Sep 17 00:00:00 2001 From: Hayden <64056131+hay-kot@users.noreply.github.com> Date: Tue, 16 May 2023 10:25:13 -0800 Subject: [PATCH 23/27] please the typing gods --- .../user_recipe_tests/test_recipe_crud.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/integration_tests/user_recipe_tests/test_recipe_crud.py b/tests/integration_tests/user_recipe_tests/test_recipe_crud.py index 9ad1281aac4..03aee6944f6 100644 --- a/tests/integration_tests/user_recipe_tests/test_recipe_crud.py +++ b/tests/integration_tests/user_recipe_tests/test_recipe_crud.py @@ -434,18 +434,18 @@ def test_get_random_order(api_client: TestClient, unique_user: utils.TestUser): assert response.status_code == 201 assert json.loads(response.text) == slug - goodparams = {"page": 1, "perPage": -1, "orderBy": "random", "paginationSeed": "abcdefg"} + goodparams: dict[str, int | str] = {"page": 1, "perPage": -1, "orderBy": "random", "paginationSeed": "abcdefg"} response = api_client.get(api_routes.recipes, params=goodparams, headers=unique_user.token) assert response.status_code == 200 - seed1_params = {"page": 1, "perPage": -1, "orderBy": "random", "paginationSeed": "abcdefg"} - seed2_params = {"page": 1, "perPage": -1, "orderBy": "random", "paginationSeed": "gfedcba"} + seed1_params: dict[str, int | str] = {"page": 1, "perPage": -1, "orderBy": "random", "paginationSeed": "abcdefg"} + seed2_params: dict[str, int | str] = {"page": 1, "perPage": -1, "orderBy": "random", "paginationSeed": "gfedcba"} data1 = api_client.get(api_routes.recipes, params=seed1_params, headers=unique_user.token).json() data2 = api_client.get(api_routes.recipes, params=seed2_params, headers=unique_user.token).json() data1_new = api_client.get(api_routes.recipes, params=seed1_params, headers=unique_user.token).json() assert data1["items"][0]["slug"] != data2["items"][0]["slug"] # new seed -> new order assert data1["items"][0]["slug"] == data1_new["items"][0]["slug"] # same seed -> same order - badparams = {"page": 1, "perPage": -1, "orderBy": "random"} + badparams: dict[str, int | str] = {"page": 1, "perPage": -1, "orderBy": "random"} response = api_client.get(api_routes.recipes, params=badparams, headers=unique_user.token) assert response.status_code == 422 From 270e9608eaf7a018600a7267d18f2fc53d03e31d Mon Sep 17 00:00:00 2001 From: Hayden <64056131+hay-kot@users.noreply.github.com> Date: Tue, 16 May 2023 10:26:08 -0800 Subject: [PATCH 24/27] hack to make query parameters throw correct exc --- mealie/routes/recipe/recipe_crud_routes.py | 5 ++-- mealie/schema/make_dependable.py | 34 ++++++++++++++++++++++ 2 files changed, 37 insertions(+), 2 deletions(-) create mode 100644 mealie/schema/make_dependable.py diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index ce3538b7f54..0e149551110 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -22,6 +22,7 @@ from mealie.routes._base import BaseCrudController, controller from mealie.routes._base.mixins import HttpRepo from mealie.routes._base.routers import MealieCrudRoute, UserAPIRouter +from mealie.schema.make_dependable import make_dependable from mealie.schema.cookbook.cookbook import ReadCookBook from mealie.schema.recipe import Recipe, RecipeImageTypes, ScrapeRecipe from mealie.schema.recipe.recipe import CreateRecipe, CreateRecipeByUrlBulk, RecipeLastMade, RecipeSummary @@ -237,8 +238,8 @@ def create_recipe_from_zip(self, temp_path=Depends(temporary_zip_path), archive: def get_all( self, request: Request, - q: PaginationQuery = Depends(), - search_query: RecipeSearchQuery = Depends(), + q: PaginationQuery = Depends(make_dependable(PaginationQuery)), + search_query: RecipeSearchQuery = Depends(make_dependable(RecipeSearchQuery)), categories: list[UUID4 | str] | None = Query(None), tags: list[UUID4 | str] | None = Query(None), tools: list[UUID4 | str] | None = Query(None), diff --git a/mealie/schema/make_dependable.py b/mealie/schema/make_dependable.py new file mode 100644 index 00000000000..c7254cc78c9 --- /dev/null +++ b/mealie/schema/make_dependable.py @@ -0,0 +1,34 @@ +from inspect import signature + +from fastapi.exceptions import HTTPException, ValidationError + + +def make_dependable(cls): + """ + Pydantic BaseModels are very powerful because we get lots of validations and type checking right out of the box. + FastAPI can accept a BaseModel as a route Dependency and it will automatically handle things like documentation + and error handling. However, if we define custom validators then the errors they raise are not handled, leading + to HTTP 500's being returned. + + To better understand this issue, you can visit https://github.com/tiangolo/fastapi/issues/1474 for context. + + A workaround proposed there adds a classmethod which attempts to init the BaseModel and handles formatting of + any raised ValidationErrors, custom or otherwise. However, this means essentially duplicating the class's + signature. This function automates the creation of a workaround method with a matching signature so that you + can avoid code duplication. + + usage: + async def fetch(thing_request: ThingRequest = Depends(make_dependable(ThingRequest))): + """ + + def init_cls_and_handle_errors(*args, **kwargs): + try: + signature(init_cls_and_handle_errors).bind(*args, **kwargs) + return cls(*args, **kwargs) + except ValidationError as e: + for error in e.errors(): + error["loc"] = ["query"] + list(error["loc"]) + raise HTTPException(422, detail=e.errors()) from None + + init_cls_and_handle_errors.__signature__ = signature(cls) + return init_cls_and_handle_errors From fe1f3167524b29bbb3394dd76e6d5f8951245e4d Mon Sep 17 00:00:00 2001 From: Hayden <64056131+hay-kot@users.noreply.github.com> Date: Tue, 16 May 2023 10:30:39 -0800 Subject: [PATCH 25/27] ruff --- mealie/routes/recipe/recipe_crud_routes.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mealie/routes/recipe/recipe_crud_routes.py b/mealie/routes/recipe/recipe_crud_routes.py index 0e149551110..6c462d30fca 100644 --- a/mealie/routes/recipe/recipe_crud_routes.py +++ b/mealie/routes/recipe/recipe_crud_routes.py @@ -22,8 +22,8 @@ from mealie.routes._base import BaseCrudController, controller from mealie.routes._base.mixins import HttpRepo from mealie.routes._base.routers import MealieCrudRoute, UserAPIRouter -from mealie.schema.make_dependable import make_dependable from mealie.schema.cookbook.cookbook import ReadCookBook +from mealie.schema.make_dependable import make_dependable from mealie.schema.recipe import Recipe, RecipeImageTypes, ScrapeRecipe from mealie.schema.recipe.recipe import CreateRecipe, CreateRecipeByUrlBulk, RecipeLastMade, RecipeSummary from mealie.schema.recipe.recipe_asset import RecipeAsset From 06434dc455e52a8e434da2413af8205fb588f9ff Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Wed, 17 May 2023 06:34:22 +0200 Subject: [PATCH 26/27] fix validator message typo --- mealie/schema/response/pagination.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mealie/schema/response/pagination.py b/mealie/schema/response/pagination.py index 5863f88d64e..f6605ed67dd 100644 --- a/mealie/schema/response/pagination.py +++ b/mealie/schema/response/pagination.py @@ -37,7 +37,7 @@ class PaginationQuery(MealieModel): @validator("pagination_seed", always=True, pre=True) def validate_randseed(cls, pagination_seed, values): if values.get("order_by") == "random" and not pagination_seed: - raise ValueError("pagination_seed required for when orderBy is random") + raise ValueError("paginationSeed is required when orderBy is random") return pagination_seed From 43f3874c73ff6ce465f7499f8074db9315945bc4 Mon Sep 17 00:00:00 2001 From: Jacob Corn Date: Mon, 29 May 2023 06:42:10 +0200 Subject: [PATCH 27/27] black reformatting --- tests/unit_tests/repository_tests/test_recipe_repository.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/unit_tests/repository_tests/test_recipe_repository.py b/tests/unit_tests/repository_tests/test_recipe_repository.py index 1084ce98059..9cce9dd04b5 100644 --- a/tests/unit_tests/repository_tests/test_recipe_repository.py +++ b/tests/unit_tests/repository_tests/test_recipe_repository.py @@ -610,7 +610,7 @@ def test_recipe_repo_search(database: AllRepositories, unique_user: TestUser): fuzzy_result = database.recipes.page_all(pagination_query, search="Steinbuck").items assert len(fuzzy_result) == 1 assert fuzzy_result[0].name == "Steinbock Sloop" - + # Test random ordering with search pagination_query = PaginationQuery( page=1, @@ -623,4 +623,4 @@ def test_recipe_repo_search(database: AllRepositories, unique_user: TestUser): for i in range(5): pagination_query.pagination_seed = str(datetime.now()) random_ordered.append(database.recipes.page_all(pagination_query, search="soup").items) - assert not all(i == random_ordered[0] for i in random_ordered) \ No newline at end of file + assert not all(i == random_ordered[0] for i in random_ordered)