Skip to content

Commit

Permalink
feat: random sort option for front page (mealie-recipes#2363)
Browse files Browse the repository at this point in the history
* Add hook for random sorting

* Add random sorting to front page

* Add multiple tests for random sorting.

* Be extra sure that all recipes are returned.

* Too stable random. seed doesn't reach backend.

* add timestamp to useRecipeSearch

* Update randomization tests for timestamp seeding

* ruff cleanup

* pass timestamp separately in getAll

* remove debugging log items

* remove timestamp from address bar

* remove defaults from backend timestamps

* timestamp should be optional

* fix edge case: query without timestamp

* similar edge case: no timestamp in pagination

* ruff :/

* better edge case handling

* stabilize random search test w/more recipes

* better pagination seeding

* update pagination seed test

* remove redundant random/seed check

* Test for api routes to random sorting.

* please the typing gods

* hack to make query parameters throw correct exc

* ruff

* fix validator message typo

* black reformatting

---------

Co-authored-by: Hayden <[email protected]>
  • Loading branch information
jecorn and hay-kot authored May 30, 2023
1 parent 7e0d29a commit e1d3a24
Show file tree
Hide file tree
Showing 10 changed files with 202 additions and 7 deletions.
1 change: 1 addition & 0 deletions frontend/composables/recipes/use-recipe-search.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ export function useRecipeSearch(api: UserApi): UseRecipeSearchReturn {
orderBy: "name",
orderDirection: "asc",
perPage: 20,
_searchSeed: Date.now().toString(),
});

if (error) {
Expand Down
6 changes: 4 additions & 2 deletions frontend/composables/recipes/use-recipes.ts
Original file line number Diff line number Diff line change
@@ -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<Recipe[]>([]);
export const recentRecipes = ref<Recipe[]>([]);
Expand All @@ -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,
Expand Down
2 changes: 2 additions & 0 deletions frontend/lib/api/user/recipes/recipe.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ export type RecipeSearchQuery = {
page?: number;
perPage?: number;
orderBy?: string;

_searchSeed?: string;
};

export class RecipeAPI extends BaseCRUDAPI<CreateRecipe, Recipe, Recipe> {
Expand Down
7 changes: 6 additions & 1 deletion frontend/pages/index.vue
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,6 @@ export default defineComponent({
foods: toIDArray(selectedFoods.value),
tags: toIDArray(selectedTags.value),
tools: toIDArray(selectedTools.value),
// Only add the query param if it's or not default
...{
auto: state.value.auto ? undefined : "false",
Expand All @@ -239,6 +238,7 @@ export default defineComponent({
requireAllFoods: state.value.requireAllFoods,
orderBy: state.value.orderBy,
orderDirection: state.value.orderDirection,
_searchSeed: Date.now().toString()
};
}
Expand Down Expand Up @@ -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(() => {
Expand Down
15 changes: 14 additions & 1 deletion mealie/repos/repository_generic.py
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
from __future__ import annotations

import random
from collections.abc import Iterable
from math import ceil
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 Select, case, delete, func, select
from sqlalchemy.orm.session import Session
from sqlalchemy.sql import sqltypes

Expand Down Expand Up @@ -378,4 +379,16 @@ def add_pagination_to_query(self, query: Select, pagination: PaginationQuery) ->

query = query.order_by(order_attr)

elif pagination.order_by == "random":
# 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)))
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)
query = query.order_by(case_stmt)

return query.limit(pagination.per_page).offset((pagination.page - 1) * pagination.per_page), count, total_pages
5 changes: 3 additions & 2 deletions mealie/routes/recipe/recipe_crud_routes.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
from mealie.routes._base.mixins import HttpRepo
from mealie.routes._base.routers import MealieCrudRoute, UserAPIRouter
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
Expand Down Expand Up @@ -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),
Expand Down
34 changes: 34 additions & 0 deletions mealie/schema/make_dependable.py
Original file line number Diff line number Diff line change
@@ -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
10 changes: 9 additions & 1 deletion mealie/schema/response/pagination.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -23,6 +23,7 @@ class RecipeSearchQuery(MealieModel):
require_all_tools: bool = False
require_all_foods: bool = False
search: str | None
_search_seed: str | None = None


class PaginationQuery(MealieModel):
Expand All @@ -31,6 +32,13 @@ class PaginationQuery(MealieModel):
order_by: str = "created_at"
order_direction: OrderDirection = OrderDirection.desc
query_filter: str | None = 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("paginationSeed is required when orderBy is random")
return pagination_seed


class PaginationBase(GenericModel, Generic[DataT]):
Expand Down
25 changes: 25 additions & 0 deletions tests/integration_tests/user_recipe_tests/test_recipe_crud.py
Original file line number Diff line number Diff line change
Expand Up @@ -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: 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: 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: 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
104 changes: 104 additions & 0 deletions tests/unit_tests/repository_tests/test_recipe_repository.py
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
from datetime import datetime
from typing import cast

from mealie.repos.repository_factory import AllRepositories
Expand Down Expand Up @@ -200,6 +201,20 @@ 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",
pagination_seed=str(datetime.now()),
order_direction=OrderDirection.asc,
)
random_ordered = []
for i in range(5):
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)


def test_recipe_repo_pagination_by_tags(database: AllRepositories, unique_user: TestUser):
slug1, slug2 = (random_string(10) for _ in range(2))
Expand Down Expand Up @@ -279,6 +294,21 @@ 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",
pagination_seed=str(datetime.now()),
order_direction=OrderDirection.asc,
)
random_ordered = []
for i in range(5):
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)


def test_recipe_repo_pagination_by_tools(database: AllRepositories, unique_user: TestUser):
slug1, slug2 = (random_string(10) for _ in range(2))
Expand Down Expand Up @@ -360,6 +390,21 @@ 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",
pagination_seed=str(datetime.now()),
order_direction=OrderDirection.asc,
)
random_ordered = []
for i in range(5):
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)


def test_recipe_repo_pagination_by_foods(database: AllRepositories, unique_user: TestUser):
slug1, slug2 = (random_string(10) for _ in range(2))
Expand Down Expand Up @@ -430,6 +475,20 @@ 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",
pagination_seed=str(datetime.now()),
order_direction=OrderDirection.asc,
)
random_ordered = []
for i in range(5):
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)


def test_recipe_repo_search(database: AllRepositories, unique_user: TestUser):
recipes = [
Expand Down Expand Up @@ -461,6 +520,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:
Expand Down Expand Up @@ -520,3 +610,17 @@ 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,
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 = 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)

0 comments on commit e1d3a24

Please sign in to comment.