Skip to content

Commit

Permalink
ENH: message: show reason for InvalidKey exception
Browse files Browse the repository at this point in the history
if the user wants to add a field with an invalid key, the reason was not
clear
now the error message says that either
the key is not allowed in the harmonization configuration
or the key does not match the regular expression (for extra keys), e.g.
 is not lower case etc.
  • Loading branch information
wagner-intevation committed Sep 6, 2023
1 parent 8bb7035 commit dc758e6
Show file tree
Hide file tree
Showing 5 changed files with 30 additions and 16 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ CHANGELOG
### Configuration

### Core
- `intelmq.lib.message`: For invalid message keys, add a hint on the failure to the exception: not allowed by configuration or not matching regular expression (PR#2398 by Sebastian Wagner).
- `intelmq.lib.exceptions.InvalidKey`: Add optional parameter `additional_text` (PR#2398 by Sebastian Wagner).

### Development

Expand Down
4 changes: 2 additions & 2 deletions intelmq/lib/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ def __init__(self, key: str, value: str, reason: Any = None, object: bytes = Non

class InvalidKey(IntelMQHarmonizationException, KeyError):

def __init__(self, key: str):
message = "invalid key %s" % repr(key)
def __init__(self, key: str, additional_text: Optional[str] = None):
message = f"invalid key {key!r} {additional_text or ''}".strip() # remove trailing whitespace if additional_text is not given
super().__init__(message)


Expand Down
24 changes: 13 additions & 11 deletions intelmq/lib/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import re
import warnings
from collections import defaultdict
from typing import Any, Dict, Iterable, Optional, Sequence, Union
from typing import Any, Dict, Iterable, Optional, Sequence, Union, Tuple
from pkg_resources import resource_filename

import intelmq.lib.exceptions as exceptions
Expand Down Expand Up @@ -186,8 +186,9 @@ def is_valid(self, key: str, value: str, sanitize: bool = True) -> bool:
intelmq.lib.exceptions.InvalidKey: if given key is invalid.
"""
if not self.__is_valid_key(key):
raise exceptions.InvalidKey(key)
key_validation = self.__is_valid_key(key)
if not key_validation[0]:
raise exceptions.InvalidKey(key, additional_text=key_validation[1])

if value is None or value in ["", "-", "N/A"]:
return False
Expand Down Expand Up @@ -243,8 +244,9 @@ def add(self, key: str, value: str, sanitize: bool = True,
del self[key]
return

if not self.__is_valid_key(key):
raise exceptions.InvalidKey(key)
key_validation = self.__is_valid_key(key)
if not key_validation[0]:
raise exceptions.InvalidKey(key, additional_text=key_validation[1])

try:
if value in ignore:
Expand Down Expand Up @@ -330,16 +332,16 @@ def unserialize(message_string: str):
message = json.loads(message_string)
return message

def __is_valid_key(self, key: str):
def __is_valid_key(self, key: str) -> Tuple[bool, str]:
try:
class_name, subitem = self.__get_type_config(key)
except KeyError:
return False
return False, 'This key is not allowed by the harmonization configuration'
if key in self.harmonization_config or key == '__type':
return True
return True, None
if subitem:
return HARMONIZATION_KEY_FORMAT.match(key)
return False
return HARMONIZATION_KEY_FORMAT.match(key), f'Does not match regular expression {HARMONIZATION_KEY_FORMAT.pattern}'
return False, 'This key is not allowed by the harmonization configuration'

def __is_valid_value(self, key: str, value: str):
if key == '__type':
Expand Down Expand Up @@ -569,7 +571,7 @@ def __init__(self, message: Union[dict, tuple] = (), auto: bool = False,
if isinstance(message, Event):
super().__init__({}, auto, harmonization)
for key, value in message.items():
if self._Message__is_valid_key(key):
if self._Message__is_valid_key(key)[0]:
self.add(key, value, sanitize=False)
else:
super().__init__(message, auto, harmonization)
Expand Down
7 changes: 7 additions & 0 deletions intelmq/tests/lib/test_exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,13 @@ def test_MissingDependencyError(self):
self.assertIn(repr(depname), exc)
self.assertTrue(exc.endswith(" %s" % additional))

def test_invalid_key(self):
"""
Check intelmq.lib.exceptions.InvalidKey
"""
exc = excs.InvalidKey('test_key', additional_text='TEST').args[0]
self.assertTrue(exc.endswith(' TEST'))


if __name__ == '__main__': # pragma: no cover
unittest.main()
9 changes: 6 additions & 3 deletions intelmq/tests/lib/test_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -168,8 +168,10 @@ def test_invalid_type2(self):
def test_report_invalid_key(self):
""" Test if report raises InvalidKey for invalid key in add(). """
report = self.new_report()
with self.assertRaises(exceptions.InvalidKey):
with self.assertRaises(exceptions.InvalidKey) as cm:
report.add('invalid', 0)
self.assertIn('not allowed', cm.exception.args[0])


def test_report_add_raw(self):
""" Test if report can add raw value. """
Expand Down Expand Up @@ -764,10 +766,11 @@ def test_invalid_harm_key(self):
message.Event(harmonization={'event': {'foo.bar.': {}}})

def test_invalid_extra_key_name(self):
""" Test if error is raised if an extra field name is invalid. """
""" Test if error is raised when an extra field name is invalid and error message is included in exception. """
event = message.Event(harmonization=HARM)
with self.assertRaises(exceptions.InvalidKey):
with self.assertRaises(exceptions.InvalidKey) as cm:
event.add('extra.foo-', 'bar')
self.assertIn('Does not match regular expression', cm.exception.args[0])


class TestReport(unittest.TestCase):
Expand Down

0 comments on commit dc758e6

Please sign in to comment.