Skip to content

Commit

Permalink
Fix: Incorrect Query Prefixing for Embedded Models in Query Construct…
Browse files Browse the repository at this point in the history
…ion (#657)

* Correct query prefixing for embedded models to prevent field concatenation

* keeping linter happy, adding test

---------

Co-authored-by: Steve Lorello <[email protected]>
Co-authored-by: slorello89 <[email protected]>
  • Loading branch information
3 people authored Nov 27, 2024
1 parent 6b521d5 commit d0e3a68
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 11 deletions.
25 changes: 14 additions & 11 deletions aredis_om/model/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,7 +309,7 @@ def score_field(self) -> str:
class ExpressionProxy:
def __init__(self, field: ModelField, parents: List[Tuple[str, "RedisModel"]]):
self.field = field
self.parents = parents
self.parents = parents.copy() # Ensure a copy is stored

def __eq__(self, other: Any) -> Expression: # type: ignore[override]
return Expression(
Expand Down Expand Up @@ -387,13 +387,14 @@ def __getattr__(self, item):
attr = getattr(embedded_cls, item)
else:
attr = getattr(outer_type, item)

if isinstance(attr, self.__class__):
# Clone the parents to ensure isolation
new_parents = self.parents.copy()
new_parent = (self.field.alias, outer_type)
if new_parent not in attr.parents:
attr.parents.append(new_parent)
new_parents = list(set(self.parents) - set(attr.parents))
if new_parents:
attr.parents = new_parents + attr.parents
if new_parent not in new_parents:
new_parents.append(new_parent)
attr.parents = new_parents
return attr


Expand Down Expand Up @@ -632,10 +633,11 @@ def resolve_value(
value: Any,
parents: List[Tuple[str, "RedisModel"]],
) -> str:
# The 'field_name' should already include the correct prefix
result = ""
if parents:
prefix = "_".join([p[0] for p in parents])
field_name = f"{prefix}_{field_name}"
result = ""
if field_type is RediSearchFieldTypes.TEXT:
result = f"@{field_name}_fts:"
if op is Operators.EQ:
Expand Down Expand Up @@ -792,8 +794,6 @@ def resolve_redisearch_query(cls, expression: ExpressionOrNegated) -> str:

if expression.op is Operators.ALL:
if encompassing_expression_is_negated:
# TODO: Is there a use case for this, perhaps for dynamic
# scoring purposes with full-text search?
raise QueryNotSupportedError(
"You cannot negate a query for all results."
)
Expand Down Expand Up @@ -827,6 +827,11 @@ def resolve_redisearch_query(cls, expression: ExpressionOrNegated) -> str:
f"or an expression enclosed in parentheses. Docs: {ERRORS_URL}#E7"
)

if isinstance(expression.left, ModelField) and expression.parents:
# Build field_name using the specific parents for this expression
prefix = "_".join([p[0] for p in expression.parents])
field_name = f"{prefix}_{field_name}"

right = expression.right

if isinstance(right, Expression) or isinstance(right, NegatedExpression):
Expand All @@ -842,8 +847,6 @@ def resolve_redisearch_query(cls, expression: ExpressionOrNegated) -> str:

if isinstance(right, NegatedExpression):
result += "-"
# We're handling the RediSearch operator in this call ("-"), so resolve the
# inner expression instead of the NegatedExpression.
right = right.expression

result += f"({cls.resolve_redisearch_query(right)})"
Expand Down
16 changes: 16 additions & 0 deletions tests/test_json_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1157,3 +1157,19 @@ class TestLiterals(JsonModel):
await item.save()
rematerialized = await TestLiterals.find(TestLiterals.flavor == "pumpkin").first()
assert rematerialized.pk == item.pk


@py_test_mark_asyncio
async def test_merged_model_error():
class Player(EmbeddedJsonModel):
username: str = Field(index=True)

class Game(JsonModel):
player1: Optional[Player]
player2: Optional[Player]

q = Game.find(
(Game.player1.username == "username") | (Game.player2.username == "username")
)
print(q.query)
assert q.query == "(@player1_username:{username})| (@player2_username:{username})"

0 comments on commit d0e3a68

Please sign in to comment.