[18763] Improve content filter expression parameters checks and verbosity. #3565

jsan-rt · 2023-06-02T09:15:17Z

Description

This PR adds an additional check to verify that the expression_parameters vector provided when creating a ContentFilteredTopic math with the limits established in the DomainParticipantQoS. It also fixes a wrong length comparison when deserializing that same vector.

@Mergifyio backport 2.10.x 2.9.x 2.6.x

Related documentation PR: eProsima/Fast-DDS-docs#510

Contributor Checklist

Commit messages follow the project guidelines.
The code follows the style guidelines of this project.
Tests that thoroughly check the new feature have been added/Regression tests checking the bug and its fix have been added; the added tests pass locally
[NA] Any new/modified methods have been properly documented using Doxygen.
Changes are ABI compatible.
Changes are API compatible.
[NA] New feature has been added to the versions.md file (if applicable).
[NA] New feature has been documented/Current behavior is correctly described in the documentation.
Applicable backports have been included in the description.

Reviewer Checklist

The PR has a milestone assigned.
Check contributor checklist is correct.
Check CI results: changes do not issue any warning.
Check CI results: failing tests are unrelated with the changes.

jsan-rt · 2023-06-03T15:37:05Z

@richiprosima please test this

jsan-rt · 2023-06-05T06:48:54Z

@richiprosima please test this

src/cpp/fastdds/core/policy/ParameterSerializer.hpp

src/cpp/fastdds/domain/DomainParticipantImpl.cpp

src/cpp/fastdds/topic/ContentFilteredTopicImpl.cpp

test/unittest/rtps/builtin/BuiltinDataSerializationTests.cpp

jsan-rt · 2023-06-14T10:47:08Z

@richiprosima please test this

JLBuenoLopez

LGTM. Please remember to include a note in the documentation stating the limit in the number of parameters in the corresponding QoS: https://fast-dds.docs.eprosima.com/en/latest/fastdds/dds_layer/core/policy/eprosimaExtensions.html#contentfilterproperty-allocationconfiguration. Also, reading the CFT section I do not find any reference to this section so I suggest including it in the same PR.

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

…ontentFilteredTopics Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

JLBuenoLopez · 2023-06-14T13:33:04Z

@richiprosima please test this

JesusPoderoso · 2023-06-14T19:01:11Z

@Mergifyio backport 2.10.x 2.9.x 2.6.x

mergify · 2023-06-14T19:01:18Z

backport 2.10.x 2.9.x 2.6.x

✅ Backports have been created

#3591 [18763] Improve content filter expression parameters checks and verbosity. (backport #3565) has been created for branch 2.10.x
#3592 [18763] Improve content filter expression parameters checks and verbosity. (backport #3565) has been created for branch 2.9.x
#3593 [18763] Improve content filter expression parameters checks and verbosity. (backport #3565) has been created for branch 2.6.x but encountered conflicts

…3565) * Refs #18763: Added max expression parameter test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed maximum parameter size checks when deserializing ContentFilteredTopics Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added test for qos expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check for expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added runtime modification to test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check to set_expression_parameters Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed signed/unsigned mismatch Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Applied suggestions Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> --------- Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> (cherry picked from commit 3073c64)

…3565) * Refs #18763: Added max expression parameter test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed maximum parameter size checks when deserializing ContentFilteredTopics Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added test for qos expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check for expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added runtime modification to test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check to set_expression_parameters Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed signed/unsigned mismatch Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Applied suggestions Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> --------- Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> (cherry picked from commit 3073c64) # Conflicts: # src/cpp/fastdds/domain/DomainParticipantImpl.cpp

…3565) (#3592) * Refs #18763: Added max expression parameter test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed maximum parameter size checks when deserializing ContentFilteredTopics Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added test for qos expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check for expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added runtime modification to test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check to set_expression_parameters Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed signed/unsigned mismatch Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Applied suggestions Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> --------- Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> (cherry picked from commit 3073c64) Co-authored-by: jsantiago-eProsima <90755661+jsantiago-eProsima@users.noreply.github.com>

) (#3591) * Refs #18763: Added max expression parameter test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed maximum parameter size checks when deserializing ContentFilteredTopics Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added test for qos expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check for expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added runtime modification to test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check to set_expression_parameters Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed signed/unsigned mismatch Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Applied suggestions Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> --------- Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> (cherry picked from commit 3073c64) Co-authored-by: jsantiago-eProsima <90755661+jsantiago-eProsima@users.noreply.github.com>

…3565) * Refs #18763: Added max expression parameter test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed maximum parameter size checks when deserializing ContentFilteredTopics Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added test for qos expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check for expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added runtime modification to test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check to set_expression_parameters Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed signed/unsigned mismatch Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Applied suggestions Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> --------- Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> (cherry picked from commit 3073c64)

) (#3593) * Refs #18763: Added max expression parameter test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed maximum parameter size checks when deserializing ContentFilteredTopics Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added test for qos expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check for expression_parameter constraints Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added runtime modification to test Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Added check to set_expression_parameters Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Fixed signed/unsigned mismatch Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> * Refs #18763: Applied suggestions Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> --------- Signed-off-by: Javier Santiago <javiersantiago@eprosima.com> (cherry picked from commit 3073c64) Co-authored-by: jsantiago-eProsima <90755661+jsantiago-eProsima@users.noreply.github.com>

JLBuenoLopez added this to the v2.11.0 milestone Jun 5, 2023

JLBuenoLopez linked an issue Jun 5, 2023 that may be closed by this pull request

ContentFilteredTopic in sources does not comply with the participant qos of allocation().content_filter.expression_parameters #3553

Closed

1 task

JLBuenoLopez added doc-pending Issue or PR which is pending to be documented and removed doc-pending Issue or PR which is pending to be documented labels Jun 13, 2023

JLBuenoLopez suggested changes Jun 13, 2023

View reviewed changes

JLBuenoLopez approved these changes Jun 14, 2023

View reviewed changes

jsan-rt mentioned this pull request Jun 14, 2023

[18763] Added Expression Parameter maximum values clarification eProsima/Fast-DDS-docs#510

Merged

jsan-rt added 8 commits June 14, 2023 15:30

Refs #18763: Added max expression parameter test

674e7aa

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

Refs #18763: Fixed maximum parameter size checks when deserializing C…

50673d6

…ontentFilteredTopics Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

Refs #18763: Added test for qos expression_parameter constraints

307ac7b

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

Refs #18763: Added check for expression_parameter constraints

00f055f

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

Refs #18763: Added runtime modification to test

49b64d6

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

Refs #18763: Added check to set_expression_parameters

7c76426

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

Refs #18763: Fixed signed/unsigned mismatch

76bcd82

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

Refs #18763: Applied suggestions

c26670e

Signed-off-by: Javier Santiago <javiersantiago@eprosima.com>

jsan-rt force-pushed the bugfix/parameter_limits_check branch from 552376d to c26670e Compare June 14, 2023 13:30

JLBuenoLopez added ci-pending PR which CI is running and removed doc-pending Issue or PR which is pending to be documented labels Jun 14, 2023

JesusPoderoso added ready-to-merge Ready to be merged. CI and changes have been reviewed and approved. and removed ci-pending PR which CI is running labels Jun 14, 2023

JLBuenoLopez merged commit 3073c64 into master Jun 15, 2023

JLBuenoLopez deleted the bugfix/parameter_limits_check branch June 15, 2023 05:35

mergify bot mentioned this pull request Jun 15, 2023

[18763] Improve content filter expression parameters checks and verbosity. (backport #3565) #3591

Merged

This was referenced Jun 15, 2023

[18763] Improve content filter expression parameters checks and verbosity. (backport #3565) #3592

Merged

[18763] Improve content filter expression parameters checks and verbosity. (backport #3565) #3593

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[18763] Improve content filter expression parameters checks and verbosity. #3565

[18763] Improve content filter expression parameters checks and verbosity. #3565

jsan-rt commented Jun 2, 2023 •

edited by JLBuenoLopez

Loading

jsan-rt commented Jun 3, 2023

jsan-rt commented Jun 5, 2023

jsan-rt commented Jun 14, 2023

JLBuenoLopez left a comment

JLBuenoLopez commented Jun 14, 2023

JesusPoderoso commented Jun 14, 2023

mergify bot commented Jun 14, 2023 •

edited

Loading

[18763] Improve content filter expression parameters checks and verbosity. #3565

[18763] Improve content filter expression parameters checks and verbosity. #3565

Conversation

jsan-rt commented Jun 2, 2023 • edited by JLBuenoLopez Loading

Description

Contributor Checklist

Reviewer Checklist

jsan-rt commented Jun 3, 2023

jsan-rt commented Jun 5, 2023

jsan-rt commented Jun 14, 2023

JLBuenoLopez left a comment

Choose a reason for hiding this comment

JLBuenoLopez commented Jun 14, 2023

JesusPoderoso commented Jun 14, 2023

mergify bot commented Jun 14, 2023 • edited Loading

✅ Backports have been created

jsan-rt commented Jun 2, 2023 •

edited by JLBuenoLopez

Loading

mergify bot commented Jun 14, 2023 •

edited

Loading