diff --git a/src/sentry/search/eap/spans/filter_aliases.py b/src/sentry/search/eap/spans/filter_aliases.py index 84784ad209ca0b..23a92fa71a076b 100644 --- a/src/sentry/search/eap/spans/filter_aliases.py +++ b/src/sentry/search/eap/spans/filter_aliases.py @@ -83,51 +83,89 @@ def semver_filter_converter(params: SnubaParams, search_filter: SearchFilter) -> organization_id = params.organization_id if organization_id is None: raise ValueError("organization is a required param") - # We explicitly use `raw_value` here to avoid converting wildcards to shell values - if not isinstance(search_filter.value.raw_value, str): - raise InvalidSearchQuery( - f"{search_filter.key.name}: Invalid value: {search_filter.value.raw_value}. Expected a semver version." - ) - version: str = search_filter.value.raw_value + operator: str = search_filter.operator - # Note that we sort this such that if we end up fetching more than - # MAX_SEMVER_SEARCH_RELEASES, we will return the releases that are closest to - # the passed filter. - order_by = Release.SEMVER_COLS - if operator.startswith("<"): - order_by = list(map(_flip_field_sort, order_by)) - qs = ( - Release.objects.filter_by_semver( - organization_id, - parse_semver(version, operator), - project_ids=params.project_ids, - ) - .values_list("version", flat=True) - .order_by(*order_by)[: constants.MAX_SEARCH_RELEASES] - ) - versions = list(qs) - final_operator: Literal["IN", "NOT IN"] = "IN" - if len(versions) == constants.MAX_SEARCH_RELEASES: - # We want to limit how many versions we pass through to Snuba. If we've hit - # the limit, make an extra query and see whether the inverse has fewer ids. - # If so, we can do a NOT IN query with these ids instead. Otherwise, we just - # do our best. - operator = constants.OPERATOR_NEGATION_MAP[operator] - # Note that the `order_by` here is important for index usage. Postgres seems - # to seq scan with this query if the `order_by` isn't included, so we - # include it even though we don't really care about order for this query - qs_flipped = ( - Release.objects.filter_by_semver(organization_id, parse_semver(version, operator)) - .order_by(*map(_flip_field_sort, order_by)) - .values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] + # Handle IN operator by processing each version separately + if operator in ["IN", "NOT IN"]: + raw_versions = search_filter.value.raw_value + if not isinstance(raw_versions, list): + raw_versions_list = [raw_versions] + else: + raw_versions_list = raw_versions + + all_versions = set() + for version_item in raw_versions_list: + try: + if not isinstance(version_item, str): + continue + # For each version in the IN clause, create a separate query using = operator + individual_qs = ( + Release.objects.filter_by_semver( + organization_id, + parse_semver(version_item, "="), + project_ids=params.project_ids, + ) + .values_list("version", flat=True) + .order_by(*Release.SEMVER_COLS)[: constants.MAX_SEARCH_RELEASES] + ) + all_versions.update(individual_qs) + except InvalidSearchQuery: + # Skip invalid semver versions in the IN clause + continue + + versions = list(all_versions) + final_op_in: Literal["IN", "NOT IN"] = "IN" if operator == "IN" else "NOT IN" + else: + # Original logic for non-IN operators + # We explicitly use `raw_value` here to avoid converting wildcards to shell values + if not isinstance(search_filter.value.raw_value, str): + raise InvalidSearchQuery( + f"{search_filter.key.name}: Invalid value: {search_filter.value.raw_value}. Expected a semver version." + ) + version_str: str = search_filter.value.raw_value + + # Note that we sort this such that if we end up fetching more than + # MAX_SEMVER_SEARCH_RELEASES, we will return the releases that are closest to + # the passed filter. + order_by = Release.SEMVER_COLS + if operator.startswith("<"): + order_by = list(map(_flip_field_sort, order_by)) + qs = ( + Release.objects.filter_by_semver( + organization_id, + parse_semver(version_str, operator), + project_ids=params.project_ids, + ) + .values_list("version", flat=True) + .order_by(*order_by)[: constants.MAX_SEARCH_RELEASES] ) + versions = list(qs) + final_op_other: Literal["IN", "NOT IN"] = "IN" + + # Apply the optimization logic for non-IN operators only + if len(versions) == constants.MAX_SEARCH_RELEASES: + # We want to limit how many versions we pass through to Snuba. If we've hit + # the limit, make an extra query and see whether the inverse has fewer ids. + # If so, we can do a NOT IN query with these ids instead. Otherwise, we just + # do our best. + operator = constants.OPERATOR_NEGATION_MAP[operator] + # Note that the `order_by` here is important for index usage. Postgres seems + # to seq scan with this query if the `order_by` isn't included, so we + # include it even though we don't really care about order for this query + qs_flipped = ( + Release.objects.filter_by_semver( + organization_id, parse_semver(version_str, operator) + ) + .order_by(*map(_flip_field_sort, order_by)) + .values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] + ) - exclude_versions = list(qs_flipped) - if exclude_versions and len(exclude_versions) < len(versions): - # Do a negative search instead - final_operator = "NOT IN" - versions = exclude_versions + exclude_versions = list(qs_flipped) + if exclude_versions and len(exclude_versions) < len(versions): + # Do a negative search instead + final_op_other = "NOT IN" + versions = exclude_versions if not validate_snuba_array_parameter(versions): raise InvalidSearchQuery( @@ -138,7 +176,13 @@ def semver_filter_converter(params: SnubaParams, search_filter: SearchFilter) -> # XXX: Just return a filter that will return no results if we have no versions versions = [constants.SEMVER_EMPTY_RELEASE] - return [SearchFilter(SearchKey(constants.RELEASE_ALIAS), final_operator, SearchValue(versions))] + return [ + SearchFilter( + SearchKey(constants.RELEASE_ALIAS), + final_op_in if operator in ["IN", "NOT IN"] else final_op_other, + SearchValue(versions), + ) + ] def semver_package_filter_converter( @@ -181,26 +225,61 @@ def semver_build_filter_converter( if organization_id is None: raise ValueError("organization is a required param") - if not isinstance(search_filter.value.raw_value, str): - raise InvalidSearchQuery( - f"{search_filter.key.name}: Invalid value: {search_filter.value.raw_value}. Expected a semver build." - ) - build: str = search_filter.value.raw_value + operator = search_filter.operator - operator, negated = handle_operator_negation(search_filter.operator) - try: - django_op = constants.OPERATOR_TO_DJANGO[operator] - except KeyError: - raise InvalidSearchQuery("Invalid operation 'IN' for semantic version filter.") - versions = list( - Release.objects.filter_by_semver_build( - organization_id, - django_op, - build, - project_ids=params.project_ids, - negated=negated, - ).values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] - ) + # Handle IN operator by processing each build separately + if operator in ["IN", "NOT IN"]: + raw_builds = search_filter.value.raw_value + if not isinstance(raw_builds, list): + raw_builds_list = [raw_builds] + else: + raw_builds_list = raw_builds + + all_versions = set() + for build_item in raw_builds_list: + try: + if not isinstance(build_item, str): + continue + # For each build in the IN clause, create a separate query using = operator + op, negated = handle_operator_negation("=") + django_op = constants.OPERATOR_TO_DJANGO[op] + individual_qs = Release.objects.filter_by_semver_build( + organization_id, + django_op, + build_item, + project_ids=params.project_ids, + negated=negated, + ).values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] + all_versions.update(individual_qs) + except (InvalidSearchQuery, KeyError): + # Skip invalid build values in the IN clause + continue + + versions = list(all_versions) + final_op_build_in = "IN" if operator == "IN" else "NOT IN" + else: + # Original logic for non-IN operators + if not isinstance(search_filter.value.raw_value, str): + raise InvalidSearchQuery( + f"{search_filter.key.name}: Invalid value: {search_filter.value.raw_value}. Expected a semver build." + ) + build_str: str = search_filter.value.raw_value + + operator, negated = handle_operator_negation(search_filter.operator) + try: + django_op = constants.OPERATOR_TO_DJANGO[operator] + except KeyError: + raise InvalidSearchQuery("Invalid operation 'IN' for semantic version filter.") + versions = list( + Release.objects.filter_by_semver_build( + organization_id, + django_op, + build_str, + project_ids=params.project_ids, + negated=negated, + ).values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] + ) + final_op_build_other = "IN" if not validate_snuba_array_parameter(versions): raise InvalidSearchQuery( @@ -211,7 +290,13 @@ def semver_build_filter_converter( # XXX: Just return a filter that will return no results if we have no versions versions = [constants.SEMVER_EMPTY_RELEASE] - return [SearchFilter(SearchKey(constants.RELEASE_ALIAS), "IN", SearchValue(versions))] + return [ + SearchFilter( + SearchKey(constants.RELEASE_ALIAS), + final_op_build_in if operator in ["IN", "NOT IN"] else final_op_build_other, + SearchValue(versions), + ) + ] def trace_filter_converter(params: SnubaParams, search_filter: SearchFilter) -> list[SearchFilter]: diff --git a/src/sentry/search/events/datasets/filter_aliases.py b/src/sentry/search/events/datasets/filter_aliases.py index 60ba3f8c7bc7f3..105ba1760b02f4 100644 --- a/src/sentry/search/events/datasets/filter_aliases.py +++ b/src/sentry/search/events/datasets/filter_aliases.py @@ -182,47 +182,82 @@ def semver_filter_converter( if builder.params.organization is None: raise ValueError("organization is a required param") organization_id: int = builder.params.organization.id - # We explicitly use `raw_value` here to avoid converting wildcards to shell values - version: str = search_filter.value.raw_value operator: str = search_filter.operator - # Note that we sort this such that if we end up fetching more than - # MAX_SEMVER_SEARCH_RELEASES, we will return the releases that are closest to - # the passed filter. - order_by = Release.SEMVER_COLS - if operator.startswith("<"): - order_by = list(map(_flip_field_sort, order_by)) - qs = ( - Release.objects.filter_by_semver( - organization_id, - parse_semver(version, operator), - project_ids=builder.params.project_ids, - ) - .values_list("version", flat=True) - .order_by(*order_by)[: constants.MAX_SEARCH_RELEASES] - ) - versions = list(qs) - final_operator = Op.IN - if len(versions) == constants.MAX_SEARCH_RELEASES: - # We want to limit how many versions we pass through to Snuba. If we've hit - # the limit, make an extra query and see whether the inverse has fewer ids. - # If so, we can do a NOT IN query with these ids instead. Otherwise, we just - # do our best. - operator = constants.OPERATOR_NEGATION_MAP[operator] - # Note that the `order_by` here is important for index usage. Postgres seems - # to seq scan with this query if the `order_by` isn't included, so we - # include it even though we don't really care about order for this query - qs_flipped = ( - Release.objects.filter_by_semver(organization_id, parse_semver(version, operator)) - .order_by(*map(_flip_field_sort, order_by)) - .values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] + # Handle IN operator by processing each version separately + if operator in ["IN", "NOT IN"]: + raw_versions = search_filter.value.raw_value + if not isinstance(raw_versions, list): + raw_versions = [raw_versions] + + all_versions = set() + for version in raw_versions: + try: + if not isinstance(version, str): + continue + # For each version in the IN clause, create a separate query using = operator + individual_qs = ( + Release.objects.filter_by_semver( + organization_id, + parse_semver(version, "="), + project_ids=builder.params.project_ids, + ) + .values_list("version", flat=True) + .order_by(*Release.SEMVER_COLS)[: constants.MAX_SEARCH_RELEASES] + ) + all_versions.update(individual_qs) + except InvalidSearchQuery: + # Skip invalid semver versions in the IN clause + continue + + versions = list(all_versions) + final_op = Op.IN if operator == "IN" else Op.NOT_IN + else: + # Original logic for non-IN operators + # We explicitly use `raw_value` here to avoid converting wildcards to shell values + version_str: str = search_filter.value.raw_value + + # Note that we sort this such that if we end up fetching more than + # MAX_SEMVER_SEARCH_RELEASES, we will return the releases that are closest to + # the passed filter. + order_by = Release.SEMVER_COLS + if operator.startswith("<"): + order_by = list(map(_flip_field_sort, order_by)) + qs = ( + Release.objects.filter_by_semver( + organization_id, + parse_semver(version_str, operator), + project_ids=builder.params.project_ids, + ) + .values_list("version", flat=True) + .order_by(*order_by)[: constants.MAX_SEARCH_RELEASES] ) + versions = list(qs) + final_op = Op.IN + + # Apply the optimization logic for non-IN operators only + if len(versions) == constants.MAX_SEARCH_RELEASES: + # We want to limit how many versions we pass through to Snuba. If we've hit + # the limit, make an extra query and see whether the inverse has fewer ids. + # If so, we can do a NOT IN query with these ids instead. Otherwise, we just + # do our best. + operator = constants.OPERATOR_NEGATION_MAP[operator] + # Note that the `order_by` here is important for index usage. Postgres seems + # to seq scan with this query if the `order_by` isn't included, so we + # include it even though we don't really care about order for this query + qs_flipped = ( + Release.objects.filter_by_semver( + organization_id, parse_semver(version_str, operator) + ) + .order_by(*map(_flip_field_sort, order_by)) + .values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] + ) - exclude_versions = list(qs_flipped) - if exclude_versions and len(exclude_versions) < len(versions): - # Do a negative search instead - final_operator = Op.NOT_IN - versions = exclude_versions + exclude_versions = list(qs_flipped) + if exclude_versions and len(exclude_versions) < len(versions): + # Do a negative search instead + final_op = Op.NOT_IN + versions = exclude_versions if not validate_snuba_array_parameter(versions): raise InvalidSearchQuery( @@ -233,7 +268,7 @@ def semver_filter_converter( # XXX: Just return a filter that will return no results if we have no versions versions = [constants.SEMVER_EMPTY_RELEASE] - return Condition(builder.column("release"), final_operator, versions) + return Condition(builder.column("release"), final_op, versions) def semver_package_filter_converter( @@ -276,22 +311,56 @@ def semver_build_filter_converter( """ if builder.params.organization is None: raise ValueError("organization is a required param") - build: str = search_filter.value.raw_value - operator, negated = handle_operator_negation(search_filter.operator) - try: - django_op = constants.OPERATOR_TO_DJANGO[operator] - except KeyError: - raise InvalidSearchQuery("Invalid operation 'IN' for semantic version filter.") - versions = list( - Release.objects.filter_by_semver_build( - builder.params.organization.id, - django_op, - build, - project_ids=builder.params.project_ids, - negated=negated, - ).values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] - ) + operator = search_filter.operator + + # Handle IN operator by processing each build separately + if operator in ["IN", "NOT IN"]: + raw_builds = search_filter.value.raw_value + if not isinstance(raw_builds, list): + raw_builds = [raw_builds] + + all_versions = set() + for build_item in raw_builds: + try: + if not isinstance(build_item, str): + continue + # For each build in the IN clause, create a separate query using = operator + op, negated = handle_operator_negation("=") + django_op = constants.OPERATOR_TO_DJANGO[op] + individual_qs = Release.objects.filter_by_semver_build( + builder.params.organization.id, + django_op, + build_item, + project_ids=builder.params.project_ids, + negated=negated, + ).values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] + all_versions.update(individual_qs) + except (InvalidSearchQuery, KeyError): + # Skip invalid build values in the IN clause + continue + + versions = list(all_versions) + final_op = Op.IN if operator == "IN" else Op.NOT_IN + else: + # Original logic for non-IN operators + build_str: str = search_filter.value.raw_value + + operator, negated = handle_operator_negation(search_filter.operator) + try: + django_op = constants.OPERATOR_TO_DJANGO[operator] + except KeyError: + raise InvalidSearchQuery("Invalid operation 'IN' for semantic version filter.") + versions = list( + Release.objects.filter_by_semver_build( + builder.params.organization.id, + django_op, + build_str, + project_ids=builder.params.project_ids, + negated=negated, + ).values_list("version", flat=True)[: constants.MAX_SEARCH_RELEASES] + ) + final_op = Op.IN if not validate_snuba_array_parameter(versions): raise InvalidSearchQuery( @@ -302,7 +371,7 @@ def semver_build_filter_converter( # XXX: Just return a filter that will return no results if we have no versions versions = [constants.SEMVER_EMPTY_RELEASE] - return Condition(builder.column("release"), Op.IN, versions) + return Condition(builder.column("release"), final_op, versions) def device_class_converter( diff --git a/src/sentry/search/events/filter.py b/src/sentry/search/events/filter.py index 4418d9c5e248f8..4aa8c66ba909d6 100644 --- a/src/sentry/search/events/filter.py +++ b/src/sentry/search/events/filter.py @@ -324,47 +324,80 @@ def _semver_filter_converter( organization_id: int = params["organization_id"] project_ids: list[int] | None = params.get("project_id") - # We explicitly use `raw_value` here to avoid converting wildcards to shell values - version: str = search_filter.value.raw_value operator: str = search_filter.operator - # Note that we sort this such that if we end up fetching more than - # MAX_SEMVER_SEARCH_RELEASES, we will return the releases that are closest to - # the passed filter. - order_by = Release.SEMVER_COLS - if operator.startswith("<"): - order_by = list(map(_flip_field_sort, order_by)) - qs = ( - Release.objects.filter_by_semver( - organization_id, - parse_semver(version, operator), - project_ids=project_ids, - ) - .values_list("version", flat=True) - .order_by(*order_by)[:MAX_SEARCH_RELEASES] - ) - versions = list(qs) - final_operator = "IN" - if len(versions) == MAX_SEARCH_RELEASES: - # We want to limit how many versions we pass through to Snuba. If we've hit - # the limit, make an extra query and see whether the inverse has fewer ids. - # If so, we can do a NOT IN query with these ids instead. Otherwise, we just - # do our best. - operator = OPERATOR_NEGATION_MAP[operator] - # Note that the `order_by` here is important for index usage. Postgres seems - # to seq scan with this query if the `order_by` isn't included, so we - # include it even though we don't really care about order for this query - qs_flipped = ( - Release.objects.filter_by_semver(organization_id, parse_semver(version, operator)) - .order_by(*map(_flip_field_sort, order_by)) - .values_list("version", flat=True)[:MAX_SEARCH_RELEASES] + # Handle IN operator by processing each version separately + if operator in ["IN", "NOT IN"]: + raw_versions = search_filter.value.raw_value + if not isinstance(raw_versions, list): + raw_versions = [raw_versions] + + all_versions = set() + for version in raw_versions: + try: + # For each version in the IN clause, create a separate query using = operator + individual_qs = ( + Release.objects.filter_by_semver( + organization_id, + parse_semver(version, "="), + project_ids=project_ids, + ) + .values_list("version", flat=True) + .order_by(*Release.SEMVER_COLS)[:MAX_SEARCH_RELEASES] + ) + all_versions.update(individual_qs) + except InvalidSearchQuery: + # Skip invalid semver versions in the IN clause + continue + + versions = list(all_versions) + final_operator = "IN" if operator == "IN" else "NOT IN" + else: + # Original logic for non-IN operators + # We explicitly use `raw_value` here to avoid converting wildcards to shell values + version_str: str = search_filter.value.raw_value + + # Note that we sort this such that if we end up fetching more than + # MAX_SEMVER_SEARCH_RELEASES, we will return the releases that are closest to + # the passed filter. + order_by = Release.SEMVER_COLS + if operator.startswith("<"): + order_by = list(map(_flip_field_sort, order_by)) + qs = ( + Release.objects.filter_by_semver( + organization_id, + parse_semver(version_str, operator), + project_ids=project_ids, + ) + .values_list("version", flat=True) + .order_by(*order_by)[:MAX_SEARCH_RELEASES] ) + versions = list(qs) + final_operator = "IN" + + # Apply the optimization logic for non-IN operators only + if len(versions) == MAX_SEARCH_RELEASES: + # We want to limit how many versions we pass through to Snuba. If we've hit + # the limit, make an extra query and see whether the inverse has fewer ids. + # If so, we can do a NOT IN query with these ids instead. Otherwise, we just + # do our best. + operator = OPERATOR_NEGATION_MAP[operator] + # Note that the `order_by` here is important for index usage. Postgres seems + # to seq scan with this query if the `order_by` isn't included, so we + # include it even though we don't really care about order for this query + qs_flipped = ( + Release.objects.filter_by_semver( + organization_id, parse_semver(version_str, operator) + ) + .order_by(*map(_flip_field_sort, order_by)) + .values_list("version", flat=True)[:MAX_SEARCH_RELEASES] + ) - exclude_versions = list(qs_flipped) - if exclude_versions and len(exclude_versions) < len(versions): - # Do a negative search instead - final_operator = "NOT IN" - versions = exclude_versions + exclude_versions = list(qs_flipped) + if exclude_versions and len(exclude_versions) < len(versions): + # Do a negative search instead + final_operator = "NOT IN" + versions = exclude_versions if not versions: # XXX: Just return a filter that will return no results if we have no versions @@ -417,29 +450,59 @@ def _semver_build_filter_converter( organization_id: int = params["organization_id"] project_ids: list[int] | None = params.get("project_id") - build: str = search_filter.value.raw_value + operator = search_filter.operator - operator, negated = handle_operator_negation(search_filter.operator) - try: - django_op = OPERATOR_TO_DJANGO[operator] - except KeyError: - raise InvalidSearchQuery("Invalid operation 'IN' for semantic version filter.") + # Handle IN operator by processing each build separately + if operator in ["IN", "NOT IN"]: + raw_builds = search_filter.value.raw_value + if not isinstance(raw_builds, list): + raw_builds = [raw_builds] - versions = list( - Release.objects.filter_by_semver_build( - organization_id, - django_op, - build, - project_ids=project_ids, - negated=negated, - ).values_list("version", flat=True)[:MAX_SEARCH_RELEASES] - ) + all_versions = set() + for build in raw_builds: + try: + # For each build in the IN clause, create a separate query using = operator + op, negated = handle_operator_negation("=") + django_op = OPERATOR_TO_DJANGO[op] + individual_qs = Release.objects.filter_by_semver_build( + organization_id, + django_op, + build, + project_ids=project_ids, + negated=negated, + ).values_list("version", flat=True)[:MAX_SEARCH_RELEASES] + all_versions.update(individual_qs) + except (InvalidSearchQuery, KeyError): + # Skip invalid build values in the IN clause + continue + + versions = list(all_versions) + final_operator = "IN" if operator == "IN" else "NOT IN" + else: + # Original logic for non-IN operators + build_str: str = search_filter.value.raw_value + operator, negated = handle_operator_negation(search_filter.operator) + try: + django_op = OPERATOR_TO_DJANGO[operator] + except KeyError: + raise InvalidSearchQuery("Invalid operation 'IN' for semantic version filter.") + + versions = list( + Release.objects.filter_by_semver_build( + organization_id, + django_op, + build_str, + project_ids=project_ids, + negated=negated, + ).values_list("version", flat=True)[:MAX_SEARCH_RELEASES] + ) + final_operator = "IN" if not versions: # XXX: Just return a filter that will return no results if we have no versions versions = [SEMVER_EMPTY_RELEASE] - return ["release", "IN", versions] + return ["release", final_operator, versions] def handle_operator_negation(operator: str) -> tuple[str, bool]: diff --git a/tests/sentry/issues/endpoints/test_organization_group_index.py b/tests/sentry/issues/endpoints/test_organization_group_index.py index a0e3231e00cc88..2cc6d162efa3e9 100644 --- a/tests/sentry/issues/endpoints/test_organization_group_index.py +++ b/tests/sentry/issues/endpoints/test_organization_group_index.py @@ -1499,6 +1499,23 @@ def test_semver(self) -> None: release_3_g_2, ] + # Test IN operator - this should work after implementation + response = self.get_response( + sort_by="date", limit=10, query=f"{SEMVER_ALIAS}:[1.2.3,1.2.5]" + ) + assert response.status_code == 200, response.content + assert [int(r["id"]) for r in response.data] == [ + release_1_g_1, + release_1_g_2, + release_3_g_1, + release_3_g_2, + ] + + # Test IN operator with single value + response = self.get_response(sort_by="date", limit=10, query=f"{SEMVER_ALIAS}:[1.2.4]") + assert response.status_code == 200, response.content + assert [int(r["id"]) for r in response.data] == [release_2_g_1, release_2_g_2] + def test_release_stage(self) -> None: replaced_release = self.create_release( version="replaced_release", @@ -1689,8 +1706,23 @@ def test_semver_build(self) -> None: release_2_g_1, ] + # This currently returns 400, but should work after implementation + response = self.get_response( + sort_by="date", limit=10, query=f"{SEMVER_BUILD_ALIAS}:[123,124]" + ) + assert response.status_code == 200, response.content + assert [int(r["id"]) for r in response.data] == [ + release_1_g_1, + release_1_g_2, + release_2_g_1, + ] + + # Test IN operator with single build response = self.get_response(sort_by="date", limit=10, query=f"{SEMVER_BUILD_ALIAS}:[124]") - assert response.status_code == 400, response.content + assert response.status_code == 200, response.content + assert [int(r["id"]) for r in response.data] == [ + release_2_g_1, + ] def test_aggregate_stats_regression_test(self) -> None: self.store_event( diff --git a/tests/sentry/search/events/test_filter.py b/tests/sentry/search/events/test_filter.py index d9e43ce05edae3..671e425c8fe994 100644 --- a/tests/sentry/search/events/test_filter.py +++ b/tests/sentry/search/events/test_filter.py @@ -202,6 +202,71 @@ def test_invalid_query(self) -> None: ): _semver_filter_converter(filter, key, {"organization_id": self.organization.id}) + def test_in_operator(self) -> None: + # Test IN operator with exact versions + release_1 = self.create_release(version="test@1.2.3") + release_2 = self.create_release(version="test@1.2.4") + release_3 = self.create_release(version="test@2.0.0") + self.create_release(version="test@1.3.0") # not included + + # Test basic IN functionality + self.run_test("IN", ["1.2.3", "1.2.4"], "IN", [release_1.version, release_2.version]) + self.run_test("IN", ["1.2.3", "2.0.0"], "IN", [release_1.version, release_3.version]) + + # Test single item in IN (should work like =) + self.run_test("IN", ["1.2.3"], "IN", [release_1.version]) + + # Test empty result + self.run_test("IN", ["3.0.0"], "IN", [SEMVER_EMPTY_RELEASE]) + + def test_in_operator_with_wildcards(self) -> None: + # Test IN operator with wildcard versions + release_1 = self.create_release(version="test@1.2.3") + release_2 = self.create_release(version="test@1.2.4") + release_3 = self.create_release(version="test@1.3.0") + release_4 = self.create_release(version="test@2.0.0") + + # Test wildcards in IN + self.run_test( + "IN", ["1.2.*", "2.*"], "IN", [release_1.version, release_2.version, release_4.version] + ) + self.run_test( + "IN", ["1.*"], "IN", [release_1.version, release_2.version, release_3.version] + ) + + def test_in_operator_with_packages(self) -> None: + # Test IN operator with package names + release_1 = self.create_release(version="package1@1.2.3") + release_2 = self.create_release(version="package2@1.2.3") + release_3 = self.create_release(version="package1@1.2.4") + + # Test package-specific versions in IN + self.run_test( + "IN", ["package1@1.2.3", "package2@1.2.3"], "IN", [release_1.version, release_2.version] + ) + self.run_test( + "IN", ["package1@1.2.3", "package1@1.2.4"], "IN", [release_1.version, release_3.version] + ) + + def test_in_operator_mixed_valid_invalid(self) -> None: + # Test IN operator with mix of valid and invalid versions + release_1 = self.create_release(version="test@1.2.3") + + # Should handle valid versions and ignore invalid ones + self.run_test("IN", ["1.2.3", "invalid.version"], "IN", [release_1.version]) + + def test_in_operator_edge_cases(self) -> None: + # Test IN operator with edge cases + + # Test empty IN list + self.run_test("IN", [], "IN", [SEMVER_EMPTY_RELEASE]) + + # Test IN with all invalid versions + self.run_test("IN", ["invalid", "also.invalid"], "IN", [SEMVER_EMPTY_RELEASE]) + + # Note: NOT IN operator is more complex for semver as it needs to + # query all releases and exclude the specified ones + def test_empty(self) -> None: self.run_test(">", "1.2.3", "IN", [SEMVER_EMPTY_RELEASE]) @@ -394,15 +459,9 @@ def test_invalid_params(self) -> None: key = SEMVER_BUILD_ALIAS filter = SearchFilter(SearchKey(key), "=", SearchValue("sentry")) with pytest.raises(ValueError, match="organization_id is a required param"): - _semver_filter_converter(filter, key, None) + _semver_build_filter_converter(filter, key, None) with pytest.raises(ValueError, match="organization_id is a required param"): - _semver_filter_converter(filter, key, {"something": 1}) # type: ignore[arg-type] # intentionally bad data - - filter = SearchFilter(SearchKey(key), "IN", SearchValue("sentry")) - with pytest.raises( - InvalidSearchQuery, match="Invalid operation 'IN' for semantic version filter." - ): - _semver_filter_converter(filter, key, {"organization_id": 1}) + _semver_build_filter_converter(filter, key, {"something": 1}) # type: ignore[arg-type] # intentionally bad data def test_empty(self) -> None: self.run_test("=", "test", "IN", [SEMVER_EMPTY_RELEASE]) @@ -416,6 +475,47 @@ def test(self) -> None: self.run_test("=", "125", "IN", [SEMVER_EMPTY_RELEASE]) self.run_test("<", "125", "IN", [release.version, release_2.version, release_3.version]) + def test_in_operator(self) -> None: + # Test IN operator with build numbers + release_1 = self.create_release(version="test@1.2.3+123") + release_2 = self.create_release(version="test@1.2.4+456") + release_3 = self.create_release(version="test@1.2.5+789") + self.create_release(version="test@1.2.6+999") # not included + + # Test basic IN functionality with numeric builds + self.run_test("IN", ["123", "456"], "IN", [release_1.version, release_2.version]) + self.run_test("IN", ["123", "789"], "IN", [release_1.version, release_3.version]) + + # Test single item in IN + self.run_test("IN", ["123"], "IN", [release_1.version]) + + # Test empty result + self.run_test("IN", ["111"], "IN", [SEMVER_EMPTY_RELEASE]) + + def test_in_operator_with_wildcards(self) -> None: + # Test IN operator with wildcard builds + release_1 = self.create_release(version="test@1.2.3+test123") + release_2 = self.create_release(version="test@1.2.4+test456") + release_3 = self.create_release(version="test@1.2.5+prod123") + + # Test wildcards in IN with build codes + self.run_test( + "IN", + ["test*", "prod*"], + "IN", + [release_1.version, release_2.version, release_3.version], + ) + self.run_test("IN", ["test*"], "IN", [release_1.version, release_2.version]) + + def test_in_operator_edge_cases(self) -> None: + # Test IN operator with edge cases + + # Test empty IN list + self.run_test("IN", [], "IN", [SEMVER_EMPTY_RELEASE]) + + # Note: NOT IN operator is more complex for semver as it needs to + # query all releases and exclude the specified ones + class ParseSemverTest(unittest.TestCase): def run_test(self, version: str, operator: str, expected: SemverFilter): @@ -433,6 +533,8 @@ def test_invalid(self) -> None: match=INVALID_SEMVER_MESSAGE, ): assert parse_semver("hello", ">") is None + # The parse_semver function doesn't handle IN operations directly + # IN operations are handled at the converter level by processing each version individually with pytest.raises( InvalidSearchQuery, match="Invalid operation 'IN' for semantic version filter.",