Skip to content

Commit e01a636

Browse files
committed
Address review: tighten _require_io_setting docstring, add lookup_value=None test
1 parent 85a6789 commit e01a636

2 files changed

Lines changed: 25 additions & 26 deletions

File tree

config/graphql/base.py

Lines changed: 3 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -23,21 +23,9 @@
2323

2424

2525
def _require_io_setting(mutation_cls: type, name: str) -> Any:
26-
"""Return ``cls.IOSettings.<name>``; raise if it is missing or ``None``.
27-
28-
``IOSettings`` is config-by-subclassing — concrete ``DRFMutation`` /
29-
``DRFDeletion`` subclasses are expected to override ``model``,
30-
``serializer`` and ``graphene_model``. Without this guard, a subclass
31-
that forgets one of those surfaces as a late ``AttributeError`` /
32-
``TypeError`` inside ``mutate()``, which gets swallowed by the broad
33-
``except Exception`` and reported to the user as a generic
34-
"internal error". Failing fast with a clear message here makes the
35-
misconfiguration obvious at mutation-invocation time.
36-
"""
26+
"""Raise ``NotImplementedError`` if ``cls.IOSettings.<name>`` is missing or ``None``."""
3727
io_settings = getattr(mutation_cls, "IOSettings", None)
38-
value = (
39-
getattr(io_settings, name, None) if io_settings is not None else None
40-
)
28+
value = getattr(io_settings, name, None) if io_settings is not None else None
4129
if value is None:
4230
raise NotImplementedError(
4331
f"{mutation_cls.__name__}.IOSettings.{name} must be set by the "
@@ -222,8 +210,7 @@ def mutate(cls, root, info, *args, **kwargs):
222210
raw_value = kwargs[pk_field]
223211
if isinstance(raw_value, list):
224212
kwargs[pk_field] = [
225-
from_global_id(global_id)[1]
226-
for global_id in raw_value
213+
from_global_id(global_id)[1] for global_id in raw_value
227214
]
228215
else:
229216
logger.info(f"pk field is: {raw_value}")

opencontractserver/tests/test_security_hardening.py

Lines changed: 22 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1208,14 +1208,12 @@ def test_validation_error_list_format(self):
12081208

12091209

12101210
# ---------------------------------------------------------------------------
1211-
# DRFMutation / DRFDeletion IOSettings misconfiguration guard (issue #1360)
1211+
# DRFMutation / DRFDeletion IOSettings misconfiguration guard
12121212
# ---------------------------------------------------------------------------
12131213

12141214

12151215
class TestIOSettingsRequiredFieldsGuard(TestCase):
1216-
"""Subclasses that forget to configure IOSettings should fail with a
1217-
clear ``NotImplementedError`` rather than a late ``AttributeError`` /
1218-
``TypeError`` that pretends to be a generic "internal error"."""
1216+
"""Misconfigured IOSettings must raise ``NotImplementedError`` at mutation time."""
12191217

12201218
def test_require_io_setting_raises_when_io_settings_missing(self):
12211219
from config.graphql.base import _require_io_setting
@@ -1229,8 +1227,7 @@ class MisconfiguredMutation:
12291227
self.assertIn("model", str(ctx.exception))
12301228

12311229
def test_require_io_setting_raises_when_attribute_none(self):
1232-
"""Mirrors the inherited ``IOSettings`` on ``DRFMutation`` where
1233-
``model``/``serializer``/``graphene_model`` default to ``None``."""
1230+
"""Each of model/serializer/graphene_model must independently fail when ``None``."""
12341231
from config.graphql.base import _require_io_setting
12351232

12361233
class MisconfiguredMutation:
@@ -1255,13 +1252,28 @@ class IOSettings:
12551252
self.assertIs(_require_io_setting(ConfiguredMutation, "model"), Corpus)
12561253

12571254
def test_base_iosettings_defaults_are_none_on_mutation(self):
1258-
"""Regression guard: the base ``DRFMutation.IOSettings`` exposes
1259-
``model`` / ``graphene_model`` / ``serializer`` as ``None`` so the
1260-
runtime guard can detect a missing override. ``DRFDeletion`` now
1261-
declares ``model`` the same way (previously not declared at all)."""
1255+
"""Base ``IOSettings`` must default to ``None`` so the runtime guard can fire."""
12621256
from config.graphql.base import DRFDeletion, DRFMutation
12631257

12641258
self.assertIsNone(DRFMutation.IOSettings.model)
12651259
self.assertIsNone(DRFMutation.IOSettings.serializer)
12661260
self.assertIsNone(DRFMutation.IOSettings.graphene_model)
12671261
self.assertIsNone(DRFDeletion.IOSettings.model)
1262+
1263+
def test_drf_deletion_mutate_raises_when_lookup_value_missing(self):
1264+
"""``DRFDeletion.mutate`` must raise ``ValueError`` when the lookup arg is omitted."""
1265+
from unittest.mock import MagicMock
1266+
1267+
from config.graphql.base import DRFDeletion
1268+
1269+
class _DeleteCorpus(DRFDeletion):
1270+
class IOSettings(DRFDeletion.IOSettings):
1271+
model = Corpus
1272+
lookup_field = "id"
1273+
1274+
info = MagicMock()
1275+
info.context.user = MagicMock(is_authenticated=True)
1276+
1277+
with self.assertRaises(ValueError) as ctx:
1278+
_DeleteCorpus.mutate(None, info)
1279+
self.assertIn("id", str(ctx.exception))

0 commit comments

Comments
 (0)