Skip to content

Commit

Permalink
Added check for metadata validation (#807)
Browse files Browse the repository at this point in the history 
* Added service config check and test for special characters.

* Test only use error files and fixed config path

* Fixed styleguide

* Changed test name

* Reverted all changes

* Added display_name check and test

* Fixed mypy check

* Fixed formating

* Initialzie call validate functions and fixed error messages

* Changed error messages
  • Loading branch information
@LazeringDeath
LazeringDeath authored Aug 5, 2024
1 parent 7d1a5fe commit 692fa28
Show file tree
Hide file tree
Showing 5 changed files with 90 additions and 9 deletions.
26 changes: 24 additions & 2 deletions packages/service/ni_measurement_plugin_sdk_service/_internal/parameter/metadata.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@

SupportedEnumType = Union[Type[Enum], _EnumTypeWrapper]

_VALID_CHARS = set(" ().,;:!?-_'+")


class ParameterMetadata(NamedTuple):
"""Class that represents the metadata of parameters."""
Expand Down Expand Up @@ -58,6 +60,7 @@ class ParameterMetadata(NamedTuple):

@staticmethod
def initialize(
validate_type: bool,
display_name: str,
type: type_pb2.Field.Kind.ValueType,
repeated: bool,
Expand All @@ -67,14 +70,16 @@ def initialize(
enum_type: Optional[SupportedEnumType] = None,
) -> "ParameterMetadata":
"""Initialize ParameterMetadata with field_name."""
_validate_display_name(display_name)
underscore_display_name = display_name.replace(" ", "_")

if all(char.isalnum() or char == "_" for char in underscore_display_name):
field_name = underscore_display_name
else:
field_name = "".join(
char for char in underscore_display_name if char.isalnum() or char == "_"
)
return ParameterMetadata(
parameter_metadata = ParameterMetadata(
display_name,
type,
repeated,
Expand All @@ -84,9 +89,26 @@ def initialize(
field_name,
enum_type,
)
if validate_type:
_validate_default_value_type(parameter_metadata)
return parameter_metadata


def _validate_display_name(display_name: str) -> None:
"""Validate and raise exception if 'display_name' has invalid characters.
Raises:
ValueError: If display_name has invalid characters.
"""
if not display_name:
raise ValueError("The display name cannot be an empty string.")
elif not display_name[0].isalpha():
raise ValueError(f"The first character in display name '{display_name}' must be a letter.")
elif not all(char in _VALID_CHARS or char.isalnum() for char in display_name):
raise ValueError(f"There are invalid characters in display name '{display_name}'.")


def validate_default_value_type(parameter_metadata: ParameterMetadata) -> None:
def _validate_default_value_type(parameter_metadata: ParameterMetadata) -> None:
"""Validate and raise exception if the default value does not match the type info.
Args:
Expand Down
3 changes: 2 additions & 1 deletion packages/service/ni_measurement_plugin_sdk_service/measurement/service.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,7 @@ def configuration(
data_type_info.type_specialization, instrument_type=instrument_type, enum_type=enum_type
)
parameter = parameter_metadata.ParameterMetadata.initialize(
True,
display_name,
data_type_info.grpc_field_type,
data_type_info.repeated,
Expand All @@ -418,7 +419,6 @@ def configuration(
data_type_info.message_type,
enum_type,
)
parameter_metadata.validate_default_value_type(parameter)
self._configuration_parameter_list.append(parameter)

def _configuration(func: _F) -> _F:
Expand Down Expand Up @@ -471,6 +471,7 @@ def output(
data_type_info.type_specialization, enum_type=enum_type
)
parameter = parameter_metadata.ParameterMetadata.initialize(
False,
display_name,
data_type_info.grpc_field_type,
data_type_info.repeated,
Expand Down
25 changes: 24 additions & 1 deletion packages/service/tests/unit/test_decoder.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -176,118 +176,135 @@ def _get_grpc_serialized_data(values):
def _get_test_parameter_by_id(default_values):
parameter_by_id = {
1: ParameterMetadata.initialize(
display_name="float_data!",
validate_type=False,
display_name="float_data",
type=type_pb2.Field.TYPE_FLOAT,
repeated=False,
default_value=default_values[0],
annotations={},
),
2: ParameterMetadata.initialize(
validate_type=False,
display_name="double_data",
type=type_pb2.Field.TYPE_DOUBLE,
repeated=False,
default_value=default_values[1],
annotations={},
),
3: ParameterMetadata.initialize(
validate_type=False,
display_name="int32_data",
type=type_pb2.Field.TYPE_INT32,
repeated=False,
default_value=default_values[2],
annotations={},
),
4: ParameterMetadata.initialize(
validate_type=False,
display_name="uint32_data",
type=type_pb2.Field.TYPE_INT64,
repeated=False,
default_value=default_values[3],
annotations={},
),
5: ParameterMetadata.initialize(
validate_type=False,
display_name="int64_data",
type=type_pb2.Field.TYPE_UINT32,
repeated=False,
default_value=default_values[4],
annotations={},
),
6: ParameterMetadata.initialize(
validate_type=False,
display_name="uint64_data",
type=type_pb2.Field.TYPE_UINT64,
repeated=False,
default_value=default_values[5],
annotations={},
),
7: ParameterMetadata.initialize(
validate_type=False,
display_name="bool_data",
type=type_pb2.Field.TYPE_BOOL,
repeated=False,
default_value=default_values[6],
annotations={},
),
8: ParameterMetadata.initialize(
validate_type=False,
display_name="string_data",
type=type_pb2.Field.TYPE_STRING,
repeated=False,
default_value=default_values[7],
annotations={},
),
9: ParameterMetadata.initialize(
validate_type=False,
display_name="double_array_data",
type=type_pb2.Field.TYPE_DOUBLE,
repeated=True,
default_value=default_values[8],
annotations={},
),
10: ParameterMetadata.initialize(
validate_type=False,
display_name="float_array_data",
type=type_pb2.Field.TYPE_FLOAT,
repeated=True,
default_value=default_values[9],
annotations={},
),
11: ParameterMetadata.initialize(
validate_type=False,
display_name="int32_array_data",
type=type_pb2.Field.TYPE_INT32,
repeated=True,
default_value=default_values[10],
annotations={},
),
12: ParameterMetadata.initialize(
validate_type=False,
display_name="uint32_array_data",
type=type_pb2.Field.TYPE_UINT32,
repeated=True,
default_value=default_values[11],
annotations={},
),
13: ParameterMetadata.initialize(
validate_type=False,
display_name="int64_array_data",
type=type_pb2.Field.TYPE_INT64,
repeated=True,
default_value=default_values[12],
annotations={},
),
14: ParameterMetadata.initialize(
validate_type=False,
display_name="uint64_array_data",
type=type_pb2.Field.TYPE_UINT64,
repeated=True,
default_value=default_values[13],
annotations={},
),
15: ParameterMetadata.initialize(
validate_type=False,
display_name="bool_array_data",
type=type_pb2.Field.TYPE_BOOL,
repeated=True,
default_value=default_values[14],
annotations={},
),
16: ParameterMetadata.initialize(
validate_type=False,
display_name="string_array_data",
type=type_pb2.Field.TYPE_STRING,
repeated=True,
default_value=default_values[15],
annotations={},
),
17: ParameterMetadata.initialize(
validate_type=False,
display_name="enum_data",
type=type_pb2.Field.TYPE_ENUM,
repeated=False,
Expand All @@ -299,6 +316,7 @@ def _get_test_parameter_by_id(default_values):
enum_type=DifferentColor,
),
18: ParameterMetadata.initialize(
validate_type=False,
display_name="enum_array_data",
type=type_pb2.Field.TYPE_ENUM,
repeated=True,
Expand All @@ -310,6 +328,7 @@ def _get_test_parameter_by_id(default_values):
enum_type=DifferentColor,
),
19: ParameterMetadata.initialize(
validate_type=False,
display_name="int_enum_data",
type=type_pb2.Field.TYPE_ENUM,
repeated=False,
Expand All @@ -321,6 +340,7 @@ def _get_test_parameter_by_id(default_values):
enum_type=Countries,
),
20: ParameterMetadata.initialize(
validate_type=False,
display_name="int_enum_array_data",
type=type_pb2.Field.TYPE_ENUM,
repeated=True,
Expand All @@ -332,6 +352,7 @@ def _get_test_parameter_by_id(default_values):
enum_type=Countries,
),
21: ParameterMetadata.initialize(
validate_type=False,
display_name="xy_data",
type=type_pb2.Field.TYPE_MESSAGE,
repeated=False,
Expand All @@ -340,6 +361,7 @@ def _get_test_parameter_by_id(default_values):
message_type=xydata_pb2.DoubleXYData.DESCRIPTOR.full_name,
),
22: ParameterMetadata.initialize(
validate_type=False,
display_name="xy_data_array",
type=type_pb2.Field.TYPE_MESSAGE,
repeated=True,
Expand Down Expand Up @@ -383,6 +405,7 @@ def _get_big_message_metadata_by_id() -> Dict[int, ParameterMetadata]:
return {
i
+ 1: ParameterMetadata.initialize(
validate_type=False,
display_name=f"field{i + 1}",
type=type_pb2.Field.TYPE_DOUBLE,
repeated=False,
Expand Down
36 changes: 33 additions & 3 deletions packages/service/tests/unit/test_metadata.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@
TYPE_SPECIALIZATION_KEY,
)
from ni_measurement_plugin_sdk_service._internal.parameter import metadata
from ni_measurement_plugin_sdk_service.measurement.info import DataType, TypeSpecialization
from ni_measurement_plugin_sdk_service.measurement.info import (
DataType,
TypeSpecialization,
)


class Color(Enum):
Expand Down Expand Up @@ -90,7 +93,7 @@ def test___default_value_different_from_type___validate___raises_type_exception(
)

with pytest.raises(TypeError):
metadata.validate_default_value_type(parameter_metadata)
metadata._validate_default_value_type(parameter_metadata)


@pytest.mark.parametrize(
Expand Down Expand Up @@ -147,4 +150,31 @@ def test___default_value_same_as_type___validate___raises_no_exception(
annotations,
)

metadata.validate_default_value_type(parameter_metadata) # implicitly assert does not throw
metadata._validate_default_value_type(parameter_metadata) # implicitly assert does not throw


@pytest.mark.parametrize(
"display_name",
[
" test_string",
"_____()!",
"test@string",
"",
],
)
def test___display_name_invalid_characters___validate___raises_value_exception(display_name):
with pytest.raises(ValueError):
metadata._validate_display_name(display_name)


@pytest.mark.parametrize(
"display_name",
[
"teststring()",
"tEsT StRIng?",
"test_string!",
"Test String: -10",
],
)
def test___display_name_valid_characters___validate___raises_no_exception(display_name):
metadata._validate_display_name(display_name)
9 changes: 7 additions & 2 deletions packages/service/tests/unit/test_service.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,13 @@

from ni_measurement_plugin_sdk_service import _datatypeinfo
from ni_measurement_plugin_sdk_service._annotations import TYPE_SPECIALIZATION_KEY
from ni_measurement_plugin_sdk_service._internal.stubs.ni.protobuf.types import xydata_pb2
from ni_measurement_plugin_sdk_service.measurement.info import DataType, TypeSpecialization
from ni_measurement_plugin_sdk_service._internal.stubs.ni.protobuf.types import (
xydata_pb2,
)
from ni_measurement_plugin_sdk_service.measurement.info import (
DataType,
TypeSpecialization,
)
from ni_measurement_plugin_sdk_service.measurement.service import MeasurementService


Expand Down

0 comments on commit 692fa28

Please sign in to comment.