fix: Prevent string discrete values which are string of numbers from passing as numeric condition values. (#130)

Author: RSam-NI

nisystemlink/clients/spec/models/_condition.py

@@ -2,6 +2,7 @@
 from typing import List, Optional, Union
 
 from nisystemlink.clients.core._uplink._json_model import JsonModel
+from pydantic import StrictFloat, StrictInt, StrictStr
 
 
 class ConditionType(Enum):
@@ -43,7 +44,11 @@ class NumericConditionValue(ConditionValueBase):
     range: Optional[List[ConditionRange]] = None
     """List of condition range values."""
 
-    discrete: Optional[List[float]] = None
+    # StrictFloat and StrictInt are used here because discrete is a common property for
+    # NumericConditionValue and StringConditionValue.
+    # If float/int is used, pydantic converts string of number into float/int by default
+    # when deserializing a StringCondition JSON and misinterprets it as NumericConditionValue type.
+    discrete: Optional[List[Union[StrictFloat, StrictInt]]] = None
     """List of condition discrete values."""
 
     unit: Optional[str] = None
@@ -56,7 +61,11 @@ class StringConditionValue(ConditionValueBase):
     String conditions may only contain discrete lists of values.
     """
 
-    discrete: Optional[List[str]] = None
+    # StrictStr is used here because discrete is a common property for
+    # NumericConditionValue and StringConditionValue.
+    # If str is used, pydantic converts any datatype into string by default when deserializing a
+    # NumericCondition JSON and misinterprets it as StringConditionValue type.
+    discrete: Optional[List[StrictStr]] = None
     """List of condition discrete values."""
 
 
@@ -66,5 +75,8 @@ class Condition(JsonModel):
     name: Optional[str] = None
     """Name of the condition."""
 
+    # Ideal approach is to set the dtype here as ConditionValue and use pydantic discriminator to
+    # deserialize/serialize the JSON into correct ConditionValue sub types. But Union of dtypes is
+    # used here as the discriminator field could be none when projection is used in query API.
     value: Optional[Union[NumericConditionValue, StringConditionValue]] = None
     """Value of the condition."""

tests/integration/spec/test_spec.py

@@ -17,6 +17,7 @@
     SpecificationLimit,
     SpecificationProjection,
     SpecificationType,
+    StringConditionValue,
     UpdateSpecificationsRequest,
     UpdateSpecificationsRequestObject,
 )
@@ -83,15 +84,15 @@ def create_specs_for_query(create_specs, product):
                     value=NumericConditionValue(
                         condition_type=ConditionType.NUMERIC,
                         range=[ConditionRange(min=-25, step=20, max=85)],
+                        discrete=[2, 1.5, 21],
                         unit="C",
                     ),
                 ),
                 Condition(
                     name="Supply Voltage",
-                    value=NumericConditionValue(
-                        condition_type=ConditionType.NUMERIC,
-                        discrete=[1.3, 1.5, 1.7],
-                        unit="mV",
+                    value=StringConditionValue(
+                        condition_type=ConditionType.STRING,
+                        discrete=["1.3", "1.5", "1.7"],
                     ),
                 ),
             ],
@@ -298,6 +299,47 @@ def test__query_spec_projection_columns__columns_returned(
         assert "spec_id" in spec_columns
         assert "name" in spec_columns
 
+    def test__query_specs__returns_condition_value_type_correctly(
+        self, client: SpecClient, create_specs, create_specs_for_query, product
+    ):
+        request = QuerySpecificationsRequest(
+            product_ids=[product],
+            projection=[
+                SpecificationProjection.CONDITION_NAME,
+                SpecificationProjection.CONDITION_UNIT,
+                SpecificationProjection.CONDITION_VALUES,
+            ],
+        )
+
+        response = client.query_specs(request)
+
+        assert response.specs
+        assert len(response.specs) == 3
+        condition_1 = (
+            response.specs[1].conditions[0].value
+            if response.specs[1].conditions
+            else None
+        )
+        condition_2 = (
+            response.specs[1].conditions[1].value
+            if response.specs[1].conditions
+            else None
+        )
+        condition_1_discrete_values = (
+            [discrete for discrete in condition_1.discrete or []] if condition_1 else []
+        )
+        condition_2_discrete_values = (
+            [discrete for discrete in condition_2.discrete or []] if condition_2 else []
+        )
+        assert isinstance(condition_1, NumericConditionValue)
+        assert isinstance(condition_2, StringConditionValue)
+        assert isinstance(condition_1_discrete_values[0], int)
+        assert isinstance(condition_1_discrete_values[1], float)
+        assert isinstance(condition_1_discrete_values[2], int)
+        assert isinstance(condition_2_discrete_values[0], str)
+        assert isinstance(condition_2_discrete_values[1], str)
+        assert isinstance(condition_2_discrete_values[2], str)
+
     def test__without_condition_type_projection__query_specs__condition_type_field_is_unset(
         self, client: SpecClient, create_specs, create_specs_for_query, product
     ):