Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes matching to all when searching content in the fake client #230

Merged
merged 1 commit into from
Jan 28, 2025

Conversation

jajreidy
Copy link
Contributor

@jajreidy jajreidy commented Jan 21, 2025

Adds ability to accept [] for all when using In matching criteria. This brings fake client in line with the regular client. Current implementation returns nothing if using "[]" using the fake client vs. the full list of items using the real client.

Refers to: RHELDST-29112

@@ -123,6 +123,8 @@ def match_field_exists(_matcher, field, obj):
@visit(InMatcher)
def match_in(matcher, field, obj):
value = get_field(field, obj)
if not matcher._values:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scenario where empty In matcher/list of values includes all the applicable values for the field is only applicable to "type_ids " attribute/field in the criteria. It's not a generalized behavior. However, this change will apply this behavior to all the fields using "InMatcher"/"match_in" option in the criteria.

e.g. an empty type_ids returns the result

$ pa_tool --env stage-ro repo unit_search --REPO_ID rhel-9-for-x86_64-baseos-rpms__9 --criteria '{"limit":1,"type_ids":[]}' | grep -E '("id"|"unit_type_id")'
        "id": "RHBA-2023:6669",
         ............  
        "unit_type_id": "erratum",

However, when the one of the fields using "in" criteria is left empty, it won't return any result

$ pa_tool --env stage-ro repo unit_search --REPO_ID rhel-9-for-x86_64-baseos-rpms__9 --criteria '{"limit":1, "filters":{"unit":{"id":{"$in":[]}}}}'
[]

If I add an id in the filter, it works as expected.

$ pa_tool --env stage-ro repo unit_search --REPO_ID rhel-9-for-x86_64-baseos-rpms__9 --criteria '{"limit":1, "filters":{"unit":{"id":{"$in":["RHBA-2023:6669"]}}}}' | grep -E '("id"|"unit_type_id")'
            "id": "RHBA-2023:6669",
         ...........   
        "unit_type_id": "erratum",

Hence, the requirement leading to this fix should take type_ids/content_type_id into account and be applicable to this field only.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the explanation on this. Hopefully the adjustment will cover that.

@@ -123,6 +123,8 @@ def match_field_exists(_matcher, field, obj):
@visit(InMatcher)
def match_in(matcher, field, obj):
value = get_field(field, obj)
if field is "content_type_id" and not matcher._values:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be == and not is.

For anyone thinking "But the test passes so this must be OK": an is comparison between strings might happen to give the same result as == because the python interpreter might have interned the strings such that only one copy of the "content_type_id" string is in memory. There are no guarantees around when this will happen, and it'll depend on how the string was constructed.

Here's an example, playing around with strings showing cases where is and == might give the same or different results:

Python 3.13.1 (main, Dec  9 2024, 00:00:00) [GCC 14.2.1 20240912 (Red Hat 14.2.1-3)] on linux
Type "help", "copyright", "credits" or "license" for more information.

>>> x = "content_type_id"

>>> y = '_'.join(['content', 'type', 'id'])

>>> x == y
True

>>> x is y
False

>>> x
'content_type_id'

>>> y
'content_type_id'

>>> x is "content_type_id"
<python-input-10>:1: SyntaxWarning: "is" with 'str' literal. Did you mean "=="?
  x is "content_type_id"
True

>>> y is "content_type_id"
<python-input-11>:1: SyntaxWarning: "is" with 'str' literal. Did you mean "=="?
  y is "content_type_id"
False

# x is the same object as a new string literal
>>> id(x)
140145959136560

>>> id("content_type_id")
140145959136560

# but y is not, even though the value is equal
>>> id(y)
140145959128496

@rajulkumar rajulkumar merged commit 5db5ffd into release-engineering:master Jan 28, 2025
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants