Skip to content

Commit b9e6ae9

Browse files
pmaipmai
authored
Merge pull request #15 from PMSFIT/fix/handle-scalar-repeated-fields
Properly handle scalar repeated fields (fixes #13)
2 parents 4e505df + 30a48a3 commit b9e6ae9

File tree

1 file changed

+101
-45
lines changed

1 file changed

+101
-45
lines changed

‎qc_ositrace/checks/osirules/osirules_checker.py‎

Lines changed: 101 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -128,27 +128,47 @@ def register_issue(
128128
def evaluate_rule_condition(
129129
message: google.protobuf.message.Message, field_name: str, rule: dict
130130
) -> bool:
131-
if not message.HasField(field_name):
131+
field_descriptor = message.DESCRIPTOR.fields_by_name.get(field_name)
132+
if field_descriptor is None:
133+
logging.warning(
134+
f"Field '{field_name}' not found in message '{message.DESCRIPTOR.full_name}'. Rule evaluation skipped."
135+
)
136+
return False
137+
if field_descriptor.label == field_descriptor.LABEL_REPEATED:
138+
values = getattr(message, field_name)
139+
else:
140+
values = [getattr(message, field_name)] if message.HasField(field_name) else []
141+
if len(values) == 0:
132142
return False
133-
value = getattr(message, field_name)
134143
if "is_set" in rule:
135144
return True
136-
if "is_greater_than" in rule and value > rule["is_greater_than"]:
145+
if "is_greater_than" in rule and all(
146+
[value > rule["is_greater_than"] for value in values]
147+
):
137148
return True
138-
if (
139-
"is_greater_than_or_equal_to" in rule
140-
and value >= rule["is_greater_than_or_equal_to"]
149+
if "is_greater_than_or_equal_to" in rule and all(
150+
[value >= rule["is_greater_than_or_equal_to"] for value in values]
141151
):
142152
return True
143-
if "is_less_than" in rule and value < rule["is_less_than"]:
153+
if "is_less_than" in rule and all(
154+
[value < rule["is_less_than"] for value in values]
155+
):
144156
return True
145-
if "is_less_than_or_equal_to" in rule and value <= rule["is_less_than_or_equal_to"]:
157+
if "is_less_than_or_equal_to" in rule and all(
158+
[value <= rule["is_less_than_or_equal_to"] for value in values]
159+
):
146160
return True
147-
if "is_equal_to" in rule and value == rule["is_equal_to"]:
161+
if "is_equal_to" in rule and all(
162+
[value == rule["is_equal_to"] for value in values]
163+
):
148164
return True
149-
if "is_different_to" in rule and value != rule["is_different_to"]:
165+
if "is_different_to" in rule and all(
166+
[value != rule["is_different_to"] for value in values]
167+
):
150168
return True
151-
if "is_iso_country_code" in rule and value >= 0 and value <= 999:
169+
if "is_iso_country_code" in rule and all(
170+
[value >= 0 and value <= 999 for value in values]
171+
):
152172
return True
153173
if "is_globally_unique" in rule or "refers_to" in rule or "check_if" in rule:
154174
# Not supported within check_if, so we return False
@@ -168,7 +188,16 @@ def check_message_against_rules(
168188

169189
# Check if required fields are set
170190
for field_name, rules in field_rules.items():
171-
has_field = message.HasField(field_name)
191+
field_descriptor = message.DESCRIPTOR.fields_by_name.get(field_name)
192+
if field_descriptor is None:
193+
logging.warning(
194+
f"Field '{field_name}' not found in message '{message.DESCRIPTOR.full_name}'. Skipping rules check."
195+
)
196+
continue
197+
if field_descriptor.label == field_descriptor.LABEL_REPEATED:
198+
has_field = True
199+
else:
200+
has_field = message.HasField(field_name)
172201
for rule_uid, rule in rules:
173202
if "is_set" in rule and not has_field:
174203
register_issue(
@@ -209,8 +238,11 @@ def check_message_against_rules(
209238

210239
# Process other rules for each set field
211240
for field, value in message.ListFields():
241+
values = value if field.label == field.LABEL_REPEATED else [value]
212242
for rule_uid, rule in field_rules.get(field.name, []):
213-
if "is_greater_than" in rule and not value > rule["is_greater_than"]:
243+
if "is_greater_than" in rule and not all(
244+
[value > rule["is_greater_than"] for value in values]
245+
):
214246
register_issue(
215247
result,
216248
message,
@@ -220,9 +252,8 @@ def check_message_against_rules(
220252
IssueSeverity.ERROR,
221253
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not greater than {rule['is_greater_than']}.",
222254
)
223-
if (
224-
"is_greater_than_or_equal_to" in rule
225-
and not value >= rule["is_greater_than_or_equal_to"]
255+
if "is_greater_than_or_equal_to" in rule and not all(
256+
[value >= rule["is_greater_than_or_equal_to"] for value in values]
226257
):
227258
register_issue(
228259
result,
@@ -233,7 +264,9 @@ def check_message_against_rules(
233264
IssueSeverity.ERROR,
234265
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not greater or equal to {rule['is_greater_than_or_equal_to']}.",
235266
)
236-
if "is_less_than" in rule and not value < rule["is_less_than"]:
267+
if "is_less_than" in rule and not all(
268+
[value < rule["is_less_than"] for value in values]
269+
):
237270
register_issue(
238271
result,
239272
message,
@@ -243,9 +276,8 @@ def check_message_against_rules(
243276
IssueSeverity.ERROR,
244277
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not less than {rule['is_less_than']}.",
245278
)
246-
if (
247-
"is_less_than_or_equal_to" in rule
248-
and not value <= rule["is_less_than_or_equal_to"]
279+
if "is_less_than_or_equal_to" in rule and not all(
280+
[value <= rule["is_less_than_or_equal_to"] for value in values]
249281
):
250282
register_issue(
251283
result,
@@ -256,7 +288,9 @@ def check_message_against_rules(
256288
IssueSeverity.ERROR,
257289
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not less or equal to {rule['is_less_than_or_equal_to']}.",
258290
)
259-
if "is_equal_to" in rule and not value == rule["is_equal_to"]:
291+
if "is_equal_to" in rule and not all(
292+
[value == rule["is_equal_to"] for value in values]
293+
):
260294
register_issue(
261295
result,
262296
message,
@@ -266,7 +300,9 @@ def check_message_against_rules(
266300
IssueSeverity.ERROR,
267301
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not equal to {rule['is_equal_to']}.",
268302
)
269-
if "is_different_to" in rule and not value != rule["is_different_to"]:
303+
if "is_different_to" in rule and not all(
304+
[value != rule["is_different_to"] for value in values]
305+
):
270306
register_issue(
271307
result,
272308
message,
@@ -277,7 +313,7 @@ def check_message_against_rules(
277313
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not different from {rule['is_different_to']}.",
278314
)
279315
if "is_iso_country_code" in rule:
280-
if value > 999 or value < 0:
316+
if any([value > 999 or value < 0 for value in values]):
281317
register_issue(
282318
result,
283319
message,
@@ -287,7 +323,9 @@ def check_message_against_rules(
287323
IssueSeverity.ERROR,
288324
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not a valid numeric ISO country code (must be between 000 and 999).",
289325
)
290-
if iso3166.countries.get(value, None) is None:
326+
if any(
327+
[iso3166.countries.get(value, None) is None for value in values]
328+
):
291329
register_issue(
292330
result,
293331
message,
@@ -298,43 +336,61 @@ def check_message_against_rules(
298336
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' is not a valid numeric ISO country code (not found in ISO 3166).",
299337
)
300338
if "is_globally_unique" in rule:
301-
if value.value in id_message_map:
302-
existing_message = id_message_map[value.value]
303-
if existing_message != message:
339+
for value in values:
340+
if value.value in id_message_map:
341+
existing_message = id_message_map[value.value]
342+
if existing_message != message:
343+
register_issue(
344+
result,
345+
message,
346+
index,
347+
time,
348+
rule_uid,
349+
IssueSeverity.ERROR,
350+
description=f"Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' is not globally unique, already used by different message '{existing_message.DESCRIPTOR.full_name}'.",
351+
)
352+
if "refers_to" in rule:
353+
for value in values:
354+
if not value.DESCRIPTOR.full_name == "osi3.Identifier":
355+
logging.warning(
356+
f"Field '{field.name}' in message '{message.DESCRIPTOR.full_name}' is of type {value.DESCRIPTOR.full_name} but refers_to rule expects an osi3.Identifier message. Skipping refers_to check."
357+
)
358+
continue
359+
if not value.HasField("value"):
304360
register_issue(
305361
result,
306362
message,
307363
index,
308364
time,
309365
rule_uid,
310366
IssueSeverity.ERROR,
311-
description=f"Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' is not globally unique, already used by different message '{existing_message.DESCRIPTOR.full_name}'.",
367+
description=f"Field '{field.name}' value {value} in message '{message.DESCRIPTOR.full_name}' does not have an identifier value, hence does not refer to any existing message.",
312368
)
313-
if "refers_to" in rule:
314-
referred_message = id_message_map.get(value.value, None)
315-
if referred_message is None:
316-
register_issue(
317-
result,
318-
message,
319-
index,
320-
time,
321-
rule_uid,
322-
IssueSeverity.ERROR,
323-
description=f"Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' does not refer to any existing message.",
324-
)
325-
else:
326-
# Check if referred message matches the expected type
327-
expected_type = f"""osi3.{rule['refers_to'].strip("'")}"""
328-
if referred_message.DESCRIPTOR.full_name != expected_type:
369+
continue
370+
referred_message = id_message_map.get(value.value, None)
371+
if referred_message is None:
329372
register_issue(
330373
result,
331374
message,
332375
index,
333376
time,
334377
rule_uid,
335378
IssueSeverity.ERROR,
336-
description=f"Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' refers to message '{referred_message.DESCRIPTOR.full_name}', which does not match the expected type '{expected_type}'.",
379+
description=f"Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' does not refer to any existing message.",
337380
)
381+
else:
382+
# Check if referred message matches the expected type
383+
expected_type = f"""osi3.{rule['refers_to'].strip("'")}"""
384+
if referred_message.DESCRIPTOR.full_name != expected_type:
385+
register_issue(
386+
result,
387+
message,
388+
index,
389+
time,
390+
rule_uid,
391+
IssueSeverity.ERROR,
392+
description=f"Field '{field.name}' value {value.value} in message '{message.DESCRIPTOR.full_name}' refers to message '{referred_message.DESCRIPTOR.full_name}', which does not match the expected type '{expected_type}'.",
393+
)
338394

339395
# Recursively check nested messages
340396
if field.message_type is not None:

0 commit comments

Comments
 (0)