Skip to content

Commit e77c851

Browse files
authored
fix(Segment Membership): Percent sign in regex condition crashes segment count refresh (#7944)
1 parent 31c9dc9 commit e77c851

5 files changed

Lines changed: 62 additions & 11 deletions

File tree

api/pyproject.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ dependencies = [
4444
"drf-writable-nested>=0.6.2,<0.7.0",
4545
"django-filter>=2.4.0,<2.5.0",
4646
"flagsmith-flag-engine>=10.1.0,<11.0.0",
47-
"flagsmith-sql-flag-engine>=0.1.3,<0.2.0",
47+
"flagsmith-sql-flag-engine>=0.2.0,<0.3.0",
4848
"django-clickhouse-backend>=1.4,<2.0",
4949
"clickhouse-driver",
5050
"boto3>=1.35.95,<1.36.0",

api/segment_membership/services.py

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,12 @@
77
from django.db.backends.utils import CursorWrapper
88
from django.db.models import Q
99
from flag_engine.context.types import EvaluationContext
10-
from flagsmith_sql_flag_engine import TranslateContext, translate_segment
10+
from flagsmith_sql_flag_engine import (
11+
Binder,
12+
PyformatParamStyle,
13+
TranslateContext,
14+
translate_segment,
15+
)
1116
from flagsmith_sql_flag_engine.dialects import ClickHouseDialect
1217
from task_processor.models import Task
1318

@@ -122,13 +127,15 @@ def compute_segment_counts_for_project(
122127
return []
123128

124129
dialect = ClickHouseDialect()
130+
binder = Binder(PyformatParamStyle())
125131
select_clauses: list[str] = []
126132
for seg in segments:
127133
translate_ctx = TranslateContext(
128134
evaluation_context=EvaluationContext(
129135
environment={"key": "_count", "name": project.name}
130136
),
131137
dialect=dialect,
138+
binder=binder,
132139
)
133140
predicate = translate_segment(
134141
map_segment_to_segment_context(map_segment_to_engine(seg)),
@@ -155,7 +162,7 @@ def compute_segment_counts_for_project(
155162
return []
156163

157164
sql = "\nUNION ALL\n".join(select_clauses)
158-
cursor.execute(sql, {"env_keys": tuple(env_id_by_key)})
165+
cursor.execute(sql, {"env_keys": tuple(env_id_by_key), **binder.params})
159166
rows: list[tuple[Any, ...]] = cursor.fetchall()
160167
membership_counts: list[SegmentMembershipCount] = []
161168
for row in rows:
@@ -186,11 +193,13 @@ def get_segment_members_page(
186193
Provide identifier as `cursor` to get a page after that identifier.
187194
Provide `q` to filter to identifiers containing it (case-insensitive).
188195
"""
196+
binder = Binder(PyformatParamStyle())
189197
translate_ctx = TranslateContext(
190198
evaluation_context=EvaluationContext(
191199
environment={"key": "_members", "name": segment.project.name}
192200
),
193201
dialect=ClickHouseDialect(),
202+
binder=binder,
194203
)
195204
predicate = translate_segment(
196205
map_segment_to_segment_context(map_segment_to_engine(segment)),
@@ -208,7 +217,11 @@ def get_segment_members_page(
208217
"i.environment_id = %(env_key)s",
209218
"i.is_deleted = false",
210219
]
211-
params: dict[str, Any] = {"env_key": environment.api_key, "limit": limit}
220+
params: dict[str, Any] = {
221+
"env_key": environment.api_key,
222+
"limit": limit,
223+
**binder.params,
224+
}
212225
if cursor:
213226
conditions.append("i.identifier > %(cursor)s")
214227
params["cursor"] = cursor

api/tests/unit/segment_membership/test_unit_segment_membership_services.py

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@
55
from common.test_tools import RunTasksFixture
66
from django.db import connections
77
from django.utils import timezone
8-
from flag_engine.segments.constants import EQUAL
8+
from flag_engine.segments.constants import EQUAL, REGEX
99
from pytest_django.fixtures import SettingsWrapper
1010
from pytest_mock import MockerFixture
1111
from task_processor.models import Task
@@ -191,6 +191,13 @@ def matching_segment(segment: Segment) -> Segment:
191191
return segment
192192

193193

194+
@pytest.fixture
195+
def percent_regex_segment(segment: Segment) -> Segment:
196+
rule = SegmentRule.objects.create(segment=segment, type=SegmentRule.ALL_RULE)
197+
Condition.objects.create(rule=rule, property="foo", operator=REGEX, value="[b%]ar")
198+
return segment
199+
200+
194201
@pytest.mark.clickhouse
195202
def test_get_segment_members_page__deleted_identity__excluded(
196203
segment_membership_identities: None,
@@ -281,6 +288,37 @@ def test_compute_segment_counts_for_project__deleted_identity__excluded_from_cou
281288
assert counts[0].count == 2
282289

283290

291+
@pytest.mark.clickhouse
292+
def test_get_segment_members_page__regex_with_percent__returns_matches(
293+
segment_membership_identities: None,
294+
percent_regex_segment: Segment,
295+
environment: Environment,
296+
) -> None:
297+
# Given / When
298+
members = get_segment_members_page(
299+
percent_regex_segment, environment, cursor=None, limit=100
300+
)
301+
302+
# Then
303+
assert [member["identifier"] for member in members] == ["alice", "bob"]
304+
305+
306+
@pytest.mark.clickhouse
307+
def test_compute_segment_counts_for_project__regex_with_percent__counts_matches(
308+
segment_membership_identities: None,
309+
percent_regex_segment: Segment,
310+
project: Project,
311+
) -> None:
312+
# Given / When
313+
with connections["clickhouse"].cursor() as cursor:
314+
counts = compute_segment_counts_for_project(project, cursor)
315+
316+
# Then
317+
assert len(counts) == 1
318+
assert counts[0].segment_id == percent_regex_segment.id
319+
assert counts[0].count == 2
320+
321+
284322
@pytest.mark.clickhouse
285323
def test_get_segment_members_page__matching_identities__returns_members_ordered_by_identifier(
286324
segment_membership_identities: None,

api/uv.lock

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

docs/docs/deployment-self-hosting/observability/_events-catalogue.md

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -369,7 +369,7 @@ Attributes:
369369
### `segment_membership.compute.segment.skipped`
370370

371371
Logged at `error` from:
372-
- `api/segment_membership/services.py:138`
372+
- `api/segment_membership/services.py:145`
373373

374374
Attributes:
375375
- `project.id`
@@ -379,7 +379,7 @@ Attributes:
379379
### `segment_membership.members.segment.skipped`
380380

381381
Logged at `error` from:
382-
- `api/segment_membership/services.py:200`
382+
- `api/segment_membership/services.py:209`
383383

384384
Attributes:
385385
- `reason`

0 commit comments

Comments
 (0)