Skip to content

Commit

Permalink
feat(issue-search): support IN for semver release search
Browse files Browse the repository at this point in the history
  • Loading branch information
JoshFerge committed Sep 11, 2024
1 parent fe2a222 commit 4c5a138
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 56 deletions.
9 changes: 8 additions & 1 deletion src/sentry/search/events/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,14 @@ class ThresholdDict(TypedDict):
"IN": "NOT IN",
"NOT IN": "IN",
}
OPERATOR_TO_DJANGO = {">=": "gte", "<=": "lte", ">": "gt", "<": "lt", "=": "exact"}
OPERATOR_TO_DJANGO = {
">=": "gte",
"<=": "lte",
">": "gt",
"<": "lt",
"=": "exact",
"IN": "in",
}

MAX_SEARCH_RELEASES = 1000
SEMVER_EMPTY_RELEASE = "____SENTRY_EMPTY_RELEASE____"
Expand Down
82 changes: 39 additions & 43 deletions src/sentry/search/events/filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -385,52 +385,48 @@ 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
version: str | Sequence[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]
def get_versions(v: str, op: str) -> tuple[str, list[str]]:
order_by = Release.SEMVER_COLS
if op.startswith("<"):
order_by = list(map(_flip_field_sort, order_by))
qs = (
Release.objects.filter_by_semver(
organization_id,
parse_semver(v, op),
project_ids=project_ids,
)
.values_list("version", flat=True)
.order_by(*order_by)[: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

if not versions:
# XXX: Just return a filter that will return no results if we have no versions
versions = [SEMVER_EMPTY_RELEASE]

return ["release", final_operator, versions]
versions = list(qs)
final_operator = "IN"
if len(versions) == MAX_SEARCH_RELEASES:
op = OPERATOR_NEGATION_MAP[op]
qs_flipped = (
Release.objects.filter_by_semver(organization_id, parse_semver(v, op))
.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):
final_operator = "NOT IN"
versions = exclude_versions
if not versions:
versions = [SEMVER_EMPTY_RELEASE]
return final_operator, versions

if isinstance(version, str):
final_operator, versions = get_versions(version, operator)
return ["release", final_operator, versions]
else:
semver_filters = []
for v in version:
_, versions = get_versions(v, operator)
semver_filters.extend(versions)
return ["release", "IN", semver_filters]


def _semver_package_filter_converter(
Expand Down
25 changes: 13 additions & 12 deletions tests/sentry/search/events/test_filter.py
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,13 @@ def test(self):
self.run_test("<=", "1.2.3", "IN", [release.version])
self.run_test("=", "1.2.4", "IN", [release_2.version])

def test_semver_in_operator(self):
release = self.create_release(version="[email protected]")
release_2 = self.create_release(version="[email protected]")
self.run_test("IN", ["1.2.3", "1.2.4"], "IN", [release.version, release_2.version])
# TODO: support not in operator
# self.run_test("NOT IN", ["1.2.3", "1.2.4"], "NOT IN", [release.version, release_2.version])

def test_invert_query(self):
# Tests that flipping the query works and uses a NOT IN. Test all operators to
# make sure the inversion works correctly.
Expand Down Expand Up @@ -436,12 +443,6 @@ def test_invalid_params(self):
with pytest.raises(ValueError, match="organization_id is a required param"):
_semver_filter_converter(filter, key, {"something": 1})

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})

def test_empty(self):
self.run_test("=", "test", "IN", [SEMVER_EMPTY_RELEASE])

Expand All @@ -456,7 +457,7 @@ def test(self):


class ParseSemverTest(unittest.TestCase):
def run_test(self, version: str, operator: str, expected: SemverFilter):
def run_test(self, version: str | list[str], operator: str, expected: SemverFilter):
semver_filter = parse_semver(version, operator)
assert semver_filter == expected

Expand All @@ -471,11 +472,6 @@ def test_invalid(self):
match=INVALID_SEMVER_MESSAGE,
):
assert parse_semver("hello", ">") is None
with pytest.raises(
InvalidSearchQuery,
match="Invalid operation 'IN' for semantic version filter.",
):
assert parse_semver("1.2.3.4", "IN") is None

def test_normal(self):
self.run_test("1", ">", SemverFilter("gt", [1, 0, 0, 0, 1, ""]))
Expand All @@ -493,6 +489,11 @@ def test_wildcard(self):
self.run_test("[email protected].*", "=", SemverFilter("exact", [1, 2, 3], "sentry"))
self.run_test("1.X", "=", SemverFilter("exact", [1]))

def test_in(self):
self.run_test("1.2.3.4", "IN", SemverFilter("in", [1, 2, 3, 4, 1, ""]))
# TODO: support not in operator
# self.run_test("1.2.3.4", "NOT IN", SemverFilter("not in", [1, 2, 3, 4, 1, ""]))


def _cond(lhs, op, rhs):
return Condition(lhs=Column(name=lhs), op=op, rhs=rhs)
Expand Down

0 comments on commit 4c5a138

Please sign in to comment.