mealie-recipes / mealie

Mealie is a self hosted recipe manager and meal planner with a RestAPI backend and a reactive frontend application built in Vue for a pleasant user experience for the whole family. Easily add recipes into your database by providing the url and mealie will automatically import the relevant data or add a family recipe with the UI editor
https://docs.mealie.io
GNU Affero General Public License v3.0
6.56k stars 670 forks source link

[BUG] - Multiple patch requests to recipe will duplicate ingredients and steps #2808

Open RealFoxie opened 8 months ago

RealFoxie commented 8 months ago

First Check

What is the issue you are experiencing?

When doing multiple PATCH requests with a description as payload, the recipe ingredients and steps are duplicated.

Note: The bug happens with payload to set the description, but might happen with other payloads too The bug happens with a parsed ingredient, but might happen with any sort of ingredient. My foods do have extra data and the sorts. If not reproduceable, I can give more details how the data looks on my end.

Steps to Reproduce

  1. Create a recipe with an (parsed) ingredient and one step
  2. do three PATCH requests with a change for description as payload AT THE SAME TIME
  3. they will all resolve, but the recipe will have duplications of the ingredients and steps (in this case three times).

Please provide relevant logs

-

Mealie Version

Latest nightly

Deployment

Docker (Linux)

Additional Deployment Details

No response

github-actions[bot] commented 7 months ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

RealFoxie commented 7 months ago

Pretty sure this is still a bug :)

Kuchenpirat commented 7 months ago

Hey thanks @RealFoxie for redirecting some attention back here. I was able to recreate your issue. The circumstances have to be verry specific. The second request has to hit before the first request has finished being handled. And the only field i was able to use from the few i tested was the description field.

I used a little shell script:

for i in {1..3}
do
    curl -X 'PATCH' \
    'http://localhost:3000/api/<recipe-slug>?group_id=<group-id>' \
    ......
    -d '{ "description": "Test" }' & 
done

It does not matter whether the ingredients are parsed or not.

Kuchenpirat commented 7 months ago

@RealFoxie how did you encounter any of this? I am not sure how a user would run against that wall, of the request not yet being handled, before sending another one.

RealFoxie commented 7 months ago

I have my own script that updates the description to put in the recipe price for every recipe I plan on a week. Sometimes I plan a recipe two times in a week, where my script would update the description two times. Instead of doing them all concurrently in the background, I now await the various update requests to avoid this issue. This could in theory still happen if two users try to do an update action at the same time? But I'm guessing the chance the request being at the same time is very small then.

RealFoxie commented 7 months ago

I also think that the dupplications might have the same id or similar? It has been a while, but I remember that e.g. Ingredient links would be linked to all the duplicated steps, instead of just the one I chose.

RealFoxie commented 2 months ago

Is there any idea what the reason might be for this and if it is fixable? I still seem to run against this, I think because sometimes mealie runs real slow (not specifically mealie's fault), so just refreshing a page in my script my even make it happen (so user interaction firing the request two times close together instead of the script itself).

I wonder if adding something else to the patch request might fix it too (didn't try it yet). Or if it is anytime the description is passed.

michael-genson commented 2 months ago

I started looking into this, but it's a bit deeper of an issue than I was hoping for. There's some sort of db-level race condition going on and there doesn't seem to be an easy way around it. Things I've tried:

You can add this test to test_recipe_crud.py to reproduce the issue consistantly:

def test_patch_recipe_doesnt_duplicate_ingredients(api_client: TestClient, unique_user: utils.TestUser):
    """https://github.com/mealie-recipes/mealie/issues/2808"""

    slug = random_string(10)
    response = api_client.post(api_routes.recipes, json={"name": slug}, headers=unique_user.token)
    assert response.status_code == 201

    # confirm we have exactly one ingredient
    response = api_client.get(api_routes.recipes_slug(slug), headers=unique_user.token)
    assert response.status_code == 200
    recipe = response.json()
    assert len(recipe["recipeIngredient"]) == 1

    def patch_request():
        recipe_url = api_routes.recipes_slug(slug)
        data = {"description": random_string()}
        response = api_client.patch(recipe_url, json=data, headers=unique_user.token)
        return response

    # send a bunch of patch requests at once
    request_count = 10
    with ThreadPoolExecutor(max_workers=request_count) as executor:
        futures = [executor.submit(patch_request) for _ in range(request_count)]
        for future in as_completed(futures):
            response = future.result()
            assert response.status_code == 200

    # confirm we still have exactly one ingredient
    response = api_client.get(api_routes.recipes_slug(slug), headers=unique_user.token)
    assert response.status_code == 200
    recipe = response.json()
    assert len(recipe["recipeIngredient"]) == 1
RealFoxie commented 2 months ago

Thanks a lot for the info and checking it out! Really weird issue indeed. At least it's good to know that it won't be fixed anytime soon.

RealFoxie commented 2 months ago

Thanks a lot for the info and checking it out! Really weird issue indeed. At least it's good to know that it won't be fixed anytime soon.

Who knows maybe it gets fixed randomly with a library update someday :p