From 804617995fae5dcac10c0ce281ca93f4bc3f437f Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Tue, 21 Nov 2023 11:10:07 +0100 Subject: [PATCH 1/4] improve error handling in pydantic validation --- monty/json.py | 18 +++++++++++++----- tests/test_json.py | 41 ++++++++++++++++++++++++++++++++++++++++- 2 files changed, 53 insertions(+), 6 deletions(-) diff --git a/monty/json.py b/monty/json.py index 8440a4225..b122805de 100644 --- a/monty/json.py +++ b/monty/json.py @@ -6,6 +6,7 @@ import json import os import pathlib +import traceback import types from collections import OrderedDict, defaultdict from enum import Enum @@ -257,12 +258,19 @@ def _validate_monty(cls, __input_value): if isinstance(__input_value, cls): return __input_value if isinstance(__input_value, dict): - new_obj = MontyDecoder().process_decoded(__input_value) - if isinstance(new_obj, cls): + # Do not any exception raised while deserialization since + # pydantic may handle them incorrectly. + try: + new_obj = MontyDecoder().process_decoded(__input_value) + if isinstance(new_obj, cls): + return new_obj + new_obj = cls(**__input_value) return new_obj - - new_obj = cls(**__input_value) - return new_obj + except Exception: + raise ValueError( + f"Error while deserializing {cls.__name__} " + f"object: {traceback.format_exc()}" + ) raise ValueError( f"Must provide {cls.__name__}, the as_dict form, or the proper" diff --git a/tests/test_json.py b/tests/test_json.py index 985db583e..5a0e11354 100644 --- a/tests/test_json.py +++ b/tests/test_json.py @@ -40,6 +40,16 @@ def __eq__(self, other): ) +class LimitedMSONClass(MSONable): + """An MSONable class that only accepts a limited number of options""" + + def __init__(self, a): + self.a = a + + def __eq__(self, other): + return self.a == other.a + + class GoodNestedMSONClass(MSONable): def __init__(self, a_list, b_dict, c_list_dict_list, **kwargs): assert isinstance(a_list, list) @@ -663,9 +673,10 @@ def test_redirect_settings_file(self): } def test_pydantic_integrations(self): - from pydantic import BaseModel + from pydantic import BaseModel, ValidationError global ModelWithMSONable # allow model to be deserialized in test + global LimitedMSONClass class ModelWithMSONable(BaseModel): a: GoodMSONClass @@ -746,6 +757,34 @@ class ModelWithDict(BaseModel): assert isinstance(obj.a["x"], GoodMSONClass) assert obj.a["x"].b == 1 + # check that if an MSONable object raises an exception during + # the model validation it is properly handled by pydantic + global ModelWithUnion # allow model to be deserialized in test + global ModelWithLimited # allow model to be deserialized in test + + class ModelWithLimited(BaseModel): + a: LimitedMSONClass + + class ModelWithUnion(BaseModel): + a: LimitedMSONClass | dict + + limited_dict = jsanitize(ModelWithLimited(a=LimitedMSONClass(1)), strict=True) + assert ModelWithLimited.model_validate(limited_dict) + limited_dict["a"]["b"] = 2 + with pytest.raises(ValidationError): + ModelWithLimited.model_validate(limited_dict) + + limited_union_dict = jsanitize( + ModelWithUnion(a=LimitedMSONClass(1)), strict=True + ) + validated_model = ModelWithUnion.model_validate(limited_union_dict) + assert isinstance(validated_model, ModelWithUnion) + assert isinstance(validated_model.a, LimitedMSONClass) + limited_union_dict["a"]["b"] = 2 + validated_model = ModelWithUnion.model_validate(limited_union_dict) + assert isinstance(validated_model, ModelWithUnion) + assert isinstance(validated_model.a, dict) + def test_dataclass(self): c = Coordinates([Point(1, 2), Point(3, 4)]) d = c.as_dict() From 8c69096be7cc5198566f32b7d959b1810619eaba Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Tue, 21 Nov 2023 10:27:40 +0000 Subject: [PATCH 2/4] pre-commit auto-fixes --- tests/test_files/3000_lines.txt | 2 +- tests/test_files/myfile | 1 - tests/test_files/myfile_txt | 1 - 3 files changed, 1 insertion(+), 3 deletions(-) diff --git a/tests/test_files/3000_lines.txt b/tests/test_files/3000_lines.txt index 2f8c055bc..1127304b4 100644 --- a/tests/test_files/3000_lines.txt +++ b/tests/test_files/3000_lines.txt @@ -2997,4 +2997,4 @@ 2997 2998 2999 -3000 \ No newline at end of file +3000 diff --git a/tests/test_files/myfile b/tests/test_files/myfile index 1e35ee2e0..459b7cc69 100644 --- a/tests/test_files/myfile +++ b/tests/test_files/myfile @@ -1,2 +1 @@ HelloWorld. - diff --git a/tests/test_files/myfile_txt b/tests/test_files/myfile_txt index 1e35ee2e0..459b7cc69 100644 --- a/tests/test_files/myfile_txt +++ b/tests/test_files/myfile_txt @@ -1,2 +1 @@ HelloWorld. - From da95a0b9394c9dfaf8df7753c0fdf4c6315ae528 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Mon, 29 Jan 2024 00:09:13 +0100 Subject: [PATCH 3/4] revert automatic fixes --- tests/test_files/3000_lines.txt | 2 +- tests/test_files/myfile | 1 + tests/test_files/myfile_txt | 1 + 3 files changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/test_files/3000_lines.txt b/tests/test_files/3000_lines.txt index 1127304b4..2f8c055bc 100644 --- a/tests/test_files/3000_lines.txt +++ b/tests/test_files/3000_lines.txt @@ -2997,4 +2997,4 @@ 2997 2998 2999 -3000 +3000 \ No newline at end of file diff --git a/tests/test_files/myfile b/tests/test_files/myfile index 459b7cc69..1e35ee2e0 100644 --- a/tests/test_files/myfile +++ b/tests/test_files/myfile @@ -1 +1,2 @@ HelloWorld. + diff --git a/tests/test_files/myfile_txt b/tests/test_files/myfile_txt index 459b7cc69..1e35ee2e0 100644 --- a/tests/test_files/myfile_txt +++ b/tests/test_files/myfile_txt @@ -1 +1,2 @@ HelloWorld. + From a43a19c1b9ff2a34136064ff39644f12bc32e421 Mon Sep 17 00:00:00 2001 From: Guido Petretto Date: Mon, 29 Jan 2024 00:23:49 +0100 Subject: [PATCH 4/4] increase test coverage --- monty/json.py | 4 ++-- tests/test_json.py | 5 +++++ 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/monty/json.py b/monty/json.py index 6bfdf38fe..68d46ca28 100644 --- a/monty/json.py +++ b/monty/json.py @@ -258,8 +258,8 @@ def _validate_monty(cls, __input_value): if isinstance(__input_value, cls): return __input_value if isinstance(__input_value, dict): - # Do not any exception raised while deserialization since - # pydantic may handle them incorrectly. + # Do not allow generic exceptions to be raised during deserialization + # since pydantic may handle them incorrectly. try: new_obj = MontyDecoder().process_decoded(__input_value) if isinstance(new_obj, cls): diff --git a/tests/test_json.py b/tests/test_json.py index ac2f79883..e3cba850c 100644 --- a/tests/test_json.py +++ b/tests/test_json.py @@ -689,6 +689,11 @@ class ModelWithMSONable(BaseModel): test_object = ModelWithMSONable(a=GoodMSONClass(1, 1, 1)) test_dict_object = ModelWithMSONable(a=test_object.a.as_dict()) assert test_dict_object.a.a == test_object.a.a + dict_no_class = test_object.a.as_dict() + dict_no_class.pop("@class") + dict_no_class.pop("@module") + test_dict_object = ModelWithMSONable(a=dict_no_class) + assert test_dict_object.a.a == test_object.a.a assert test_object.model_json_schema() == { "title": "ModelWithMSONable",