Add support for handling of RegularExpression definitions for string based properties #426

Kostadin-Ivanov · 2024-10-28T16:21:44Z

Add support for handling of RegularExpression definitions for string based properties.

The idea is to be able to specify a Regular Expression field, where we can define a RegExp string, which can be used for string based property specifications.

An example would be the Vehicle.VehicleIdentification.VIN which currently has definition of:

VehicleIdentification.VIN:
  datatype: string
  type: attribute
  description: 17-character Vehicle Identification Number (VIN) as defined by ISO 3779.

As per above description, there is not available RegExp or other way to validate such strings and users are "left" to deal with the validation of this field on their own.

Currently there is only min/max constraints, which are for number based properties.

With this update, we would like to allow option to add additional sort of validation (constraints) for string based values like VehicleIdentification.VIN, which can easy be used by VSS users.

As result, we are aiming to be able to have a RegExp definition for the VIN and any other VSS properties which might need such validation as follows:

VehicleIdentification.VIN:
  datatype: string
  type: attribute
  description: 17-character Vehicle Identification Number (VIN) as defined by ISO 3779.
  pattern: ^([0-9A-HJ-NPR-Z]{3})([0-9A-HJ-NPR-Z]{6})([0-9A-HJ-NPR-Z]{4}[0-9]{4})$

NOTE: the specified pattern is tested on: https://regex101.com/, with a test string of: 1CPH423GAP71E2745 and validates VIN strings as per quoted ISO 3779 on below rules:

VIN length of 17 chars where:
1.1 First 3 characters are for the VMI: 1CP
1.2 Next 6 characters are for the VDS: H423GA
1.3 Last 8 characters are for the VIS: P71E2745
The last 4 characters of the VIS part should always be numbers
Letters: I, O and Q shall not be used

If required, a better RegExp might be provided / updated using overlays by users, but the above one can be used as a general example, which covers 6 rules from the above mentioned ISO Standard.

With regards to update to the Vehicle.VehicleIdentification.VIN, we provided another Pull Request: Add RegExp pattern for VehicleIdentification.VIN property. in the COVESA VehicleSignalSpecification which currently fails, because of the extra property: pattern until this PR gets approved and released to the used by the VehiclSignalSpecification VSS-Tools.

In addition, this update also reflects and the VSS - samm exporter so the newly introduced node attribute: pattern to be properly handled in generated ESMF - SAMM aspect models with corresponding SAMM - RegularExpressionConstraint.

sschleemilch · 2024-10-29T05:25:41Z

src/vss_tools/model.py

+    # Field, used to allow definition of Regular Expression constraints
+    # for string based property nodes.
+    # Example: VSS - VehicleIdentification.VIN property
+    pattern: str | None = None


We would also need a model validator function for pattern to evaluate it. It should fail when being defined and:

datatype is not a string

default being set and is not matching

Thank you Sebastian, I will update the model.

sschleemilch · 2024-10-29T05:32:46Z

src/vss_tools/exporters/samm/helpers/vss_helper.py

    return (
        hasattr(vss_node.data, "type")
-        and vss_node.data.type in [NodeType.ACTUATOR, NodeType.SENSOR]


Since the goal seems to be to check against attributes of the VSSDataDatatype node type it can probably be simplified to:

data = node.data if isinstance(data, VSSDataDatatype): return data.max or data.min or data.pattern return False

Is VSSDataProperty not being covered on purpose?

Thank you.
I will simplify this as advised.

On the question about: VSSDataProperty - I must have missed it there.

Initially I was looking for some VSSData... class for string properties where I can add the pattern field but I just found the main one: VSSDataDatatype.

If needed, I will double check and as asked will provide a validation of the pattern to be defined only for string types.

Probably need to check them against None. So my implementation is not completely correct I suppose. At least the max and min ;-)

Sure, I will try to do my best here :)

Hello @sschleemilch , on the question about VSSDataProperty. The code above was a bit obsolete, since I had below check:

vss_node.data.type in [NodeType.ACTUATOR, NodeType.SENSOR, NodeType.ATTRIBUTE]

which I guess adds for the VSSDataProperty check. However, since I think that all: sensor, actuator and attribute are children of the VSSDataDatatype, I updated the check as you advised and now it looks like:

def has_constraints(vss_node: VSSNode) -> bool: return bool( isinstance(vss_node.data, VSSDataDatatype) and (vss_node.data.max or vss_node.data.min or vss_node.data.pattern) )

I tested this on the whole VSS spect and it seems to be fine for every node which has min/max and pattern constraints.

erikbosch · 2024-10-29T15:31:56Z

MoM: Please review

sschleemilch

Please add some tests for the pattern functionality.
Can be completely separate tests similar to how it has been done here: https://github.com/COVESA/vss-tools/pull/425/files

No need to care about exporters at that level

sschleemilch · 2024-10-30T12:55:58Z

src/vss_tools/exporters/samm/helpers/vss_helper.py

-            and vss_node.data.min is not None
-        )
+    return bool(
+        isinstance(vss_node.data, VSSDataDatatype) and (vss_node.data.max or vss_node.data.min or vss_node.data.pattern)


how does it behave if max: 0? It probably returns False but should return True. So probably needs explicit None checking for min and max

I will update it for is not None, totally forgot about the 0s :)

sschleemilch · 2024-10-30T12:58:21Z

src/vss_tools/model.py

+        3. if default value is provided it is matching the specified pattern
+        """
+        if self.pattern:
+            assert bool(type(self.pattern) is str), f"Specified pattern: '{self.pattern}' must be a string"


That pattern is a string is already checked by Pydantic and is not needed

Sure, I noticed it when I was testing it manually, but forgot to remove this check.

sschleemilch · 2024-10-30T12:59:20Z

src/vss_tools/model.py

+
+            # Datatypes.TUPLE[0] is the string name of the type.
+            allowed_for = f"It is allowed only for '{Datatypes.STRING[0]}' and '{Datatypes.STRING_ARRAY[0]}' types."
+            assert bool(


bool casting is not necessary

Ok.
I will check it and probably remove the casting.

sschleemilch · 2024-10-30T13:02:17Z

src/vss_tools/model.py

+            allowed_for = f"It is allowed only for '{Datatypes.STRING[0]}' and '{Datatypes.STRING_ARRAY[0]}' types."
+            assert bool(
+                Datatypes.get_type(self.datatype) in [Datatypes.STRING, Datatypes.STRING_ARRAY]
+            ), f"Field 'pattern' is not allowed for '{self.fqn}' of type: '{self.datatype}'. {allowed_for}"


The fqn info is already in the rendered exception and does not need to be given.

I will update the error response.

sschleemilch · 2024-10-30T13:03:13Z

src/vss_tools/model.py

+            assert bool(type(self.pattern) is str), f"Specified pattern: '{self.pattern}' must be a string"
+
+            # Datatypes.TUPLE[0] is the string name of the type.
+            allowed_for = f"It is allowed only for '{Datatypes.STRING[0]}' and '{Datatypes.STRING_ARRAY[0]}' types."


I would make this a bit shorter. Like

f"allowed: {[Datatypes.STRING[0], Datatypes.STRING_ARRAY[0]]}"

sschleemilch · 2024-10-30T13:06:08Z

src/vss_tools/model.py

+                Datatypes.get_type(self.datatype) in [Datatypes.STRING, Datatypes.STRING_ARRAY]
+            ), f"Field 'pattern' is not allowed for '{self.fqn}' of type: '{self.datatype}'. {allowed_for}"
+
+            if self.default:


if default is a list we need to iterate and do re.match on each element.

It can be something like:

check_values = [self.default] if is_array(self.default): check_values = self.default for value in check_values: assert re.match(self.pattern, value), f"...."

Thank you for this catch.

Done.

Since we are going to use patterns for string arrays, probably would never happen, but just in case I also added validation of allowed data.

Kostadin-Ivanov · 2024-10-31T08:34:17Z

Please add some tests for the pattern functionality. Can be completely separate tests similar to how it has been done here: https://github.com/COVESA/vss-tools/pull/425/files

No need to care about exporters at that level

I was thinking about this and will add some unit tests.
Thank you for the hint :)

…based properties Signed-off-by: Kostadin Ivanov (BD/TBC-BG) <[email protected]>

Kostadin-Ivanov · 2024-11-01T15:02:33Z

Hello @sschleemilch ,

I added some, I think :), integration tests to cover the validations of the usage of pattern datatype field.

Ah, I forgot to add check for wrong pattern type, i.e. a number instead of a string. Please let me know if it is OK like that, or I should add the number check, and I will do it straight away :)

Kostadin-Ivanov force-pushed the add_reg_exp_definition branch from c6f01a4 to 62a03da Compare October 28, 2024 16:24

Kostadin-Ivanov mentioned this pull request Oct 28, 2024

Add RegExp pattern for VehicleIdentification.VIN property. COVESA/vehicle_signal_specification#786

Open

sschleemilch suggested changes Oct 29, 2024

View reviewed changes

Kostadin-Ivanov force-pushed the add_reg_exp_definition branch 2 times, most recently from 8916de1 to c3269bd Compare October 30, 2024 09:51

sschleemilch suggested changes Oct 30, 2024

View reviewed changes

Kostadin-Ivanov force-pushed the add_reg_exp_definition branch 8 times, most recently from 1ec6d8a to ea3bf7d Compare November 1, 2024 14:39

Add support for handling of RegularExpression definitions for string …

81601f3

…based properties Signed-off-by: Kostadin Ivanov (BD/TBC-BG) <[email protected]>

Kostadin-Ivanov force-pushed the add_reg_exp_definition branch from ea3bf7d to 81601f3 Compare November 1, 2024 14:53

Kostadin-Ivanov requested a review from sschleemilch November 1, 2024 14:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support for handling of RegularExpression definitions for string based properties #426

Add support for handling of RegularExpression definitions for string based properties #426

Kostadin-Ivanov commented Oct 28, 2024 •

edited

Loading

sschleemilch Oct 29, 2024

Kostadin-Ivanov Oct 29, 2024

Kostadin-Ivanov Oct 30, 2024

sschleemilch Oct 29, 2024

Kostadin-Ivanov Oct 29, 2024 •

edited

Loading

sschleemilch Oct 29, 2024

Kostadin-Ivanov Oct 29, 2024

Kostadin-Ivanov Oct 30, 2024 •

edited

Loading

Kostadin-Ivanov Oct 30, 2024

erikbosch commented Oct 29, 2024

sschleemilch left a comment

sschleemilch Oct 30, 2024

Kostadin-Ivanov Oct 31, 2024 •

edited

Loading

Kostadin-Ivanov Oct 31, 2024

sschleemilch Oct 30, 2024

Kostadin-Ivanov Oct 31, 2024

Kostadin-Ivanov Oct 31, 2024

sschleemilch Oct 30, 2024

Kostadin-Ivanov Oct 31, 2024

Kostadin-Ivanov Oct 31, 2024

sschleemilch Oct 30, 2024

Kostadin-Ivanov Oct 31, 2024

Kostadin-Ivanov Oct 31, 2024

sschleemilch Oct 30, 2024

Kostadin-Ivanov Oct 31, 2024

sschleemilch Oct 30, 2024

Kostadin-Ivanov Oct 31, 2024

Kostadin-Ivanov Oct 31, 2024

Kostadin-Ivanov commented Oct 31, 2024

Kostadin-Ivanov commented Nov 1, 2024

Add support for handling of RegularExpression definitions for string based properties #426

Are you sure you want to change the base?

Add support for handling of RegularExpression definitions for string based properties #426

Conversation

Kostadin-Ivanov commented Oct 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kostadin-Ivanov Oct 29, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kostadin-Ivanov Oct 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikbosch commented Oct 29, 2024

sschleemilch left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kostadin-Ivanov Oct 31, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Kostadin-Ivanov commented Oct 31, 2024

Kostadin-Ivanov commented Nov 1, 2024

Kostadin-Ivanov commented Oct 28, 2024 •

edited

Loading

Kostadin-Ivanov Oct 29, 2024 •

edited

Loading

Kostadin-Ivanov Oct 30, 2024 •

edited

Loading

Kostadin-Ivanov Oct 31, 2024 •

edited

Loading