fix: reject an unknown field name in search text_fields#3
Merged
Merged
Conversation
search() silently skipped any text_fields entry not present in the schema
(Err(_) => continue), so a typo like text_fields:["titel"] produced no text
clause and returned {"hits":[]} -- indistinguishable from a legitimately empty
result and invisible in production. It was also inconsistent with the where/in
sections of the same function, which already error on an unknown field.
Resolve text_fields up front, once, before the per-word loop: an unknown named
field now returns a clear error ("campo text_fields '<name>' no existe"),
matching the where/in style. A legitimately empty text_fields list stays a
no-op, and a genuinely empty query still returns []. The up-front pass also
validates even when text is empty (the previous lazy resolution never ran with
zero tokens) and removes the redundant per-token get_field.
Reconcile the ClientInterface docblock: an unknown top-level query key is
ignored (serde drops it), but a named field absent from the schema is a hard
error.
Tests: rejects_unknown_text_field reproduces the silent-empty bug and now
asserts the exact error; valid_text_field_still_matches and
empty_query_still_returns_empty guard against over-reach. Validated against the
real Service Desk search path (text_fields is always getIndexes(), a subset of
the index's declared text fields -- no call-site passes an undeclared name).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
search() silently skipped any text_fields entry not present in the schema (Err(_) => continue), so a typo like text_fields:["titel"] produced no text clause and returned {"hits":[]} -- indistinguishable from a legitimately empty result and invisible in production. It was also inconsistent with the where/in sections of the same function, which already error on an unknown field.
Resolve text_fields up front, once, before the per-word loop: an unknown named field now returns a clear error ("campo text_fields '' no existe"), matching the where/in style. A legitimately empty text_fields list stays a no-op, and a genuinely empty query still returns []. The up-front pass also validates even when text is empty (the previous lazy resolution never ran with zero tokens) and removes the redundant per-token get_field.
Reconcile the ClientInterface docblock: an unknown top-level query key is ignored (serde drops it), but a named field absent from the schema is a hard error.
Tests: rejects_unknown_text_field reproduces the silent-empty bug and now asserts the exact error; valid_text_field_still_matches and empty_query_still_returns_empty guard against over-reach. Validated against the real Service Desk search path (text_fields is always getIndexes(), a subset of the index's declared text fields -- no call-site passes an undeclared name).