fix(gateway): make WriteEngine + converter raise-safe against malformed write commands
This commit is contained in:
@@ -25,6 +25,8 @@ def to_typed_value(raw, data_type: str):
|
|||||||
data_type: 'boolean' | 'int' | 'float' | 'double' | 'string'.
|
data_type: 'boolean' | 'int' | 'float' | 'double' | 'string'.
|
||||||
Raises ConversionError on any invalid input (never a bare TypeError).
|
Raises ConversionError on any invalid input (never a bare TypeError).
|
||||||
"""
|
"""
|
||||||
|
if not isinstance(data_type, str):
|
||||||
|
raise ConversionError(f'data_type must be a string, got {type(data_type).__name__}')
|
||||||
dtype = data_type.strip().lower()
|
dtype = data_type.strip().lower()
|
||||||
try:
|
try:
|
||||||
if dtype in ('bool', 'boolean'):
|
if dtype in ('bool', 'boolean'):
|
||||||
|
|||||||
@@ -88,6 +88,10 @@ class WriteEngine(SientiaMonitoring):
|
|||||||
async def write(self, cmd: dict) -> WriteResult:
|
async def write(self, cmd: dict) -> WriteResult:
|
||||||
await self._rate_limit()
|
await self._rate_limit()
|
||||||
|
|
||||||
|
missing = [k for k in ('value', 'data_type', 'server_id', 'addr') if k not in cmd]
|
||||||
|
if missing:
|
||||||
|
return await self._emit_error(cmd, 'bad_command', f'missing fields: {missing}', 'error')
|
||||||
|
|
||||||
try:
|
try:
|
||||||
typed = to_typed_value(cmd['value'], cmd['data_type'])
|
typed = to_typed_value(cmd['value'], cmd['data_type'])
|
||||||
except ConversionError as e:
|
except ConversionError as e:
|
||||||
@@ -156,7 +160,7 @@ class WriteEngine(SientiaMonitoring):
|
|||||||
return result
|
return result
|
||||||
|
|
||||||
async def _emit_success(self, cmd: dict, start: float):
|
async def _emit_success(self, cmd: dict, start: float):
|
||||||
tags = {'pod_id': self.pod_id, 'server_name': cmd['server_id']}
|
tags = {'pod_id': self.pod_id, 'server_name': cmd.get('server_id', 'unknown')}
|
||||||
await self.emit_metric(
|
await self.emit_metric(
|
||||||
metric_object=metrics.OPC_WRITE_TOTAL,
|
metric_object=metrics.OPC_WRITE_TOTAL,
|
||||||
tags={**tags, 'model_id': cmd.get('model_id', 'unknown'), 'result': 'success'},
|
tags={**tags, 'model_id': cmd.get('model_id', 'unknown'), 'result': 'success'},
|
||||||
|
|||||||
@@ -161,6 +161,46 @@ async def test_write_bad_type_returns_error_without_raising():
|
|||||||
)
|
)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_write_none_data_type_returns_error_without_raising():
|
||||||
|
eng = make_engine()
|
||||||
|
|
||||||
|
result = await eng.write(base_cmd(data_type=None))
|
||||||
|
|
||||||
|
assert result.ok is False
|
||||||
|
assert result.status == 'error'
|
||||||
|
eng.sink.update_write_status.assert_awaited_once()
|
||||||
|
eng.pool.get.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_write_missing_value_key_returns_error_without_raising():
|
||||||
|
eng = make_engine()
|
||||||
|
cmd = base_cmd()
|
||||||
|
del cmd['value']
|
||||||
|
|
||||||
|
result = await eng.write(cmd)
|
||||||
|
|
||||||
|
assert result.ok is False
|
||||||
|
assert result.status == 'error'
|
||||||
|
eng.sink.update_write_status.assert_awaited_once()
|
||||||
|
eng.pool.get.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.asyncio
|
||||||
|
async def test_write_missing_data_type_key_returns_error_without_raising():
|
||||||
|
eng = make_engine()
|
||||||
|
cmd = base_cmd()
|
||||||
|
del cmd['data_type']
|
||||||
|
|
||||||
|
result = await eng.write(cmd)
|
||||||
|
|
||||||
|
assert result.ok is False
|
||||||
|
assert result.status == 'error'
|
||||||
|
eng.sink.update_write_status.assert_awaited_once()
|
||||||
|
eng.pool.get.assert_not_awaited()
|
||||||
|
|
||||||
|
|
||||||
@pytest.mark.asyncio
|
@pytest.mark.asyncio
|
||||||
async def test_write_value_raises_maps_to_error_result_not_raised():
|
async def test_write_value_raises_maps_to_error_result_not_raised():
|
||||||
eng = make_engine()
|
eng = make_engine()
|
||||||
|
|||||||
@@ -22,3 +22,10 @@ def test_float_and_int():
|
|||||||
def test_invalid_raises_conversion_error(raw, dtype):
|
def test_invalid_raises_conversion_error(raw, dtype):
|
||||||
with pytest.raises(ConversionError):
|
with pytest.raises(ConversionError):
|
||||||
to_typed_value(raw, dtype)
|
to_typed_value(raw, dtype)
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize('dtype', [None, 123])
|
||||||
|
def test_non_string_data_type_raises_conversion_error(dtype):
|
||||||
|
# contract: never a bare AttributeError/TypeError, always ConversionError
|
||||||
|
with pytest.raises(ConversionError):
|
||||||
|
to_typed_value('5', dtype)
|
||||||
|
|||||||
Reference in New Issue
Block a user