From b2e0c51eade5fe544c0afbf2f4dbfd40bd7ade06 Mon Sep 17 00:00:00 2001 From: Michael Genson <71845777+michael-genson@users.noreply.github.com> Date: Tue, 13 Feb 2024 19:46:12 +0000 Subject: [PATCH 1/4] make sure report entries are actually saved --- mealie/services/migrations/_migration_base.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/mealie/services/migrations/_migration_base.py b/mealie/services/migrations/_migration_base.py index dd76baf982a7..bbb8d5486255 100644 --- a/mealie/services/migrations/_migration_base.py +++ b/mealie/services/migrations/_migration_base.py @@ -14,6 +14,7 @@ from mealie.schema.reports.reports import ( ReportCategory, ReportCreate, ReportEntryCreate, + ReportEntryOut, ReportOut, ReportSummary, ReportSummaryStatus, @@ -91,6 +92,7 @@ class BaseMigrator(BaseService): is_success = True is_failure = True + new_entries: list[ReportEntryOut] = [] for entry in self.report_entries: if is_failure and entry.success: is_failure = False @@ -98,7 +100,7 @@ class BaseMigrator(BaseService): if is_success and not entry.success: is_success = False - self.db.group_report_entries.create(entry) + new_entries.append(self.db.group_report_entries.create(entry)) if is_success: self.report.status = ReportSummaryStatus.success @@ -109,6 +111,7 @@ class BaseMigrator(BaseService): if not is_success and not is_failure: self.report.status = ReportSummaryStatus.partial + self.report.entries = new_entries self.db.group_reports.update(self.report.id, self.report) def migrate(self, report_name: str) -> ReportSummary: From f2e7deb5cba398679360ca5dbf4558eef955ef1f Mon Sep 17 00:00:00 2001 From: Michael Genson <71845777+michael-genson@users.noreply.github.com> Date: Tue, 13 Feb 2024 19:50:38 +0000 Subject: [PATCH 2/4] added fault tolerance for malformed recipe json --- mealie/services/migrations/mealie_alpha.py | 29 ++++++++++++++-------- 1 file changed, 19 insertions(+), 10 deletions(-) diff --git a/mealie/services/migrations/mealie_alpha.py b/mealie/services/migrations/mealie_alpha.py index 0886e8d509b3..57fc374ccb6b 100644 --- a/mealie/services/migrations/mealie_alpha.py +++ b/mealie/services/migrations/mealie_alpha.py @@ -5,6 +5,7 @@ import zipfile from pathlib import Path from mealie.schema.recipe.recipe import Recipe +from mealie.schema.reports.reports import ReportEntryCreate from ._migration_base import BaseMigrator from .utils.migration_alias import MigrationAlias @@ -55,20 +56,28 @@ class MealieAlphaMigrator(BaseMigrator): zip_file.extractall(tmpdir) temp_path = Path(tmpdir) - recipe_lookup: dict[str, Path] = {} - recipes_as_dicts = [] - for x in temp_path.rglob("**/recipes/**/[!.]*.json"): - if (y := MigrationReaders.json(x)) is not None: - recipes_as_dicts.append(y) - slug = y["slug"] - recipe_lookup[slug] = x.parent - - recipes = [self._convert_to_new_schema(x) for x in recipes_as_dicts] + recipes: list[Recipe] = [] + for recipe_json_path in temp_path.rglob("**/recipes/**/[!.]*.json"): + try: + if (recipe_as_dict := MigrationReaders.json(recipe_json_path)) is not None: + recipe = self._convert_to_new_schema(recipe_as_dict) + recipes.append(recipe) + slug = recipe_as_dict["slug"] + recipe_lookup[slug] = recipe_json_path.parent + except Exception as e: + self.logger.exception(e) + self.report_entries.append( + ReportEntryCreate( + report_id=self.report_id, + success=False, + message=f"Failed to import {recipe_json_path.name}", + exception=f"{e.__class__.__name__}: {e}", + ) + ) results = self.import_recipes_to_database(recipes) - for slug, recipe_id, status in results: if not status: continue From 72052be92f9dc52437982fc53c8cd6986310782f Mon Sep 17 00:00:00 2001 From: Michael Genson <71845777+michael-genson@users.noreply.github.com> Date: Tue, 13 Feb 2024 19:54:47 +0000 Subject: [PATCH 3/4] added test --- .../test_recipe_migrations.py | 66 +++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/tests/integration_tests/recipe_migration_tests/test_recipe_migrations.py b/tests/integration_tests/recipe_migration_tests/test_recipe_migrations.py index 7bdc1bc79b0f..7532bf3a6cbd 100644 --- a/tests/integration_tests/recipe_migration_tests/test_recipe_migrations.py +++ b/tests/integration_tests/recipe_migration_tests/test_recipe_migrations.py @@ -1,10 +1,14 @@ +import os from dataclasses import dataclass from pathlib import Path +from tempfile import TemporaryDirectory +from zipfile import ZipFile import pytest from fastapi.testclient import TestClient from mealie.schema.group.group_migration import SupportedMigrations +from mealie.schema.reports.reports import ReportEntryOut from tests import data as test_data from tests.utils import api_routes from tests.utils.assertion_helpers import assert_derserialize @@ -60,6 +64,8 @@ def test_recipe_migration(api_client: TestClient, unique_user: TestUser, mig: Mi response = api_client.get(api_routes.groups_reports_item_id(report_id), headers=unique_user.token) assert response.status_code == 200 + response_json = response.json() + assert response_json["entries"] for item in response.json()["entries"]: assert item["success"] @@ -77,3 +83,63 @@ def test_recipe_migration(api_client: TestClient, unique_user: TestUser, mig: Mi query_data = assert_derserialize(response) events = query_data["items"] assert len(events) + + +def test_bad_mealie_alpha_data_is_ignored(api_client: TestClient, unique_user: TestUser): + with TemporaryDirectory() as tmpdir: + with ZipFile(test_data.migrations_mealie) as zf: + zf.extractall(tmpdir) + + invalid_recipe_dir = os.path.join(tmpdir, "mealie_2021-Dec-08", "recipes", "invalid-recipe") + os.makedirs(invalid_recipe_dir, exist_ok=True) + invalid_json_path = os.path.join(invalid_recipe_dir, "invalid-recipe.json") + try: + with open(invalid_json_path, "w"): + pass # write nothing to the file, which is invalid JSON + except Exception: + raise Exception(os.listdir(tmpdir)) + + modified_test_data = os.path.join(tmpdir, "modified-test-data.zip") + with ZipFile(modified_test_data, "w") as zf: + for root, _, files in os.walk(tmpdir): + for file in files: + file_path = os.path.join(root, file) + zf.write(file_path, arcname=os.path.relpath(file_path, tmpdir)) + + payload = { + "migration_type": SupportedMigrations.mealie_alpha.value, + } + + file_payload = { + "archive": Path(modified_test_data).read_bytes(), + } + + response = api_client.post( + api_routes.groups_migrations, data=payload, files=file_payload, headers=unique_user.token + ) + + assert response.status_code == 200 + report_id = response.json()["id"] + + # Validate Results + response = api_client.get(api_routes.groups_reports_item_id(report_id), headers=unique_user.token) + + assert response.status_code == 200 + response_json = response.json() + assert response_json["entries"] + + failed_item = None + failed_item_count = 0 + for item in response_json["entries"]: + if item["success"]: + continue + + failed_item = item + failed_item_count += 1 + + assert failed_item + assert failed_item_count == 1 + + report_entry = ReportEntryOut.model_validate(failed_item) + assert report_entry.message == "Failed to import invalid-recipe.json" + assert report_entry.exception == "JSONDecodeError: Expecting value: line 1 column 1 (char 0)" From 9c6e3ebe5bd5606fce56225de17c1ae44c6d560b Mon Sep 17 00:00:00 2001 From: Michael Genson <71845777+michael-genson@users.noreply.github.com> Date: Tue, 13 Feb 2024 20:24:16 +0000 Subject: [PATCH 4/4] fixed new var ref --- .../recipe_migration_tests/test_recipe_migrations.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/integration_tests/recipe_migration_tests/test_recipe_migrations.py b/tests/integration_tests/recipe_migration_tests/test_recipe_migrations.py index 7532bf3a6cbd..22f6e62451a0 100644 --- a/tests/integration_tests/recipe_migration_tests/test_recipe_migrations.py +++ b/tests/integration_tests/recipe_migration_tests/test_recipe_migrations.py @@ -67,7 +67,7 @@ def test_recipe_migration(api_client: TestClient, unique_user: TestUser, mig: Mi response_json = response.json() assert response_json["entries"] - for item in response.json()["entries"]: + for item in response_json["entries"]: assert item["success"] # Validate Create Event