Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Random sort option for front page #2363

Merged
merged 28 commits into from
May 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
34ed385
Add hook for random sorting
jecorn Apr 25, 2023
0a801c4
Add random sorting to front page
jecorn Apr 25, 2023
56643c2
Add multiple tests for random sorting.
jecorn Apr 25, 2023
80f476a
Be extra sure that all recipes are returned.
jecorn Apr 25, 2023
5608f34
Too stable random. seed doesn't reach backend.
jecorn May 8, 2023
ef6fc0f
add timestamp to useRecipeSearch
jecorn May 9, 2023
aa548ec
Update randomization tests for timestamp seeding
jecorn May 9, 2023
c265521
ruff cleanup
jecorn May 9, 2023
04ddf00
pass timestamp separately in getAll
jecorn May 10, 2023
83f6e71
remove debugging log items
jecorn May 10, 2023
16b7955
remove timestamp from address bar
jecorn May 10, 2023
d37765a
remove defaults from backend timestamps
jecorn May 10, 2023
f4056c4
timestamp should be optional
jecorn May 11, 2023
112b891
fix edge case: query without timestamp
jecorn May 11, 2023
92c94ae
similar edge case: no timestamp in pagination
jecorn May 11, 2023
0bdfd6b
ruff :/
jecorn May 11, 2023
8b4f6de
better edge case handling
jecorn May 11, 2023
9fe47d7
stabilize random search test w/more recipes
jecorn May 11, 2023
ab46b88
better pagination seeding
jecorn May 16, 2023
14df0b7
update pagination seed test
jecorn May 16, 2023
88facf8
remove redundant random/seed check
jecorn May 16, 2023
71ac6e2
Test for api routes to random sorting.
jecorn May 16, 2023
ff4b657
please the typing gods
hay-kot May 16, 2023
270e960
hack to make query parameters throw correct exc
hay-kot May 16, 2023
fe1f316
ruff
hay-kot May 16, 2023
06434dc
fix validator message typo
jecorn May 17, 2023
d2aaff1
Merge branch 'mealie-next' into randomsort
jecorn May 29, 2023
43f3874
black reformatting
jecorn May 29, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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)