Skip to content

Commit 94368ed

Browse files
committed
fix(rtdb): Preserve server-side key order in order_by_key() query results
The Realtime Database server returns order_by_key() results in Firebase key order (integer-parseable keys first, numerically; then string keys lexicographically). Query.get() re-sorted this payload client-side with a pure lexicographic comparison, scrambling the server order and breaking keyset pagination (feeding the last returned key back into start_at() returned the same page forever). Skip the client-side sorter for order_by_key() and return the server-ordered payload as an OrderedDict, preserving the documented return type. Fixes #677
1 parent a7aafa2 commit 94368ed

2 files changed

Lines changed: 44 additions & 2 deletions

File tree

firebase_admin/db.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -481,8 +481,9 @@ class Query:
481481
is applied on the sorted data to produce the final result. Despite the ordering constraint,
482482
the final result is returned by the server as an unordered collection. Therefore the Query
483483
interface performs another round of sorting at the client-side before returning the results
484-
to the caller. This client-side sorted results are returned to the user as a Python
485-
OrderedDict.
484+
to the caller. The only exceptions are queries ordered by key (``order_by_key()``) or by
485+
priority (``order_by='$priority'``), for which the server-returned order is preserved as is.
486+
This client-side sorted results are returned to the user as a Python OrderedDict.
486487
"""
487488

488489
def __init__(self, **kwargs):
@@ -618,6 +619,10 @@ def get(self):
618619
FirebaseError: If an error occurs while communicating with the remote database server.
619620
"""
620621
result = self._client.body('get', self._pathurl, params=self._querystr)
622+
if isinstance(result, dict) and self._order_by == '$key':
623+
# The server already returns results in key order. Re-sorting them client-side
624+
# with a pure lexicographic comparison would scramble that order (see issue #677).
625+
return collections.OrderedDict(result)
621626
if isinstance(result, (dict, list)) and self._order_by != '$priority':
622627
return _Sorter(result, self._order_by).get()
623628
return result

tests/test_db.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,6 +1044,43 @@ def test_invalid_query_args(self):
10441044
with pytest.raises(ValueError):
10451045
db.Query(order_by='$key', client=ref._client, pathurl=ref._add_suffix(), foo='bar')
10461046

1047+
@staticmethod
1048+
def _instrument_body(query, payload, monkeypatch):
1049+
"""Stubs out the HTTP layer of a Query with a fixed server payload."""
1050+
class _StubClient:
1051+
def body(self, unused_method, unused_url, params=None):
1052+
return dict(payload)
1053+
monkeypatch.setattr(query, '_client', _StubClient())
1054+
1055+
def test_get_order_by_key_preserves_server_order(self, monkeypatch):
1056+
# Regression test for https://github.com/firebase/firebase-admin-python/issues/677
1057+
# The RTDB server returns children in Firebase key order (integer-parseable keys
1058+
# first, numerically; then string keys lexicographically). The client must not
1059+
# re-sort the payload with a pure lexicographic comparison, which would scramble
1060+
# the server order and break keyset pagination (e.g. feeding the last returned
1061+
# key back into start_at()).
1062+
query = self.ref.order_by_key().limit_to_first(4)
1063+
server_payload = collections.OrderedDict([
1064+
('123', {'myValue': True}),
1065+
('100001', {'myValue': True}),
1066+
('100002', {'myValue': True}),
1067+
('100003', {'myValue': True}),
1068+
])
1069+
self._instrument_body(query, server_payload, monkeypatch)
1070+
result = query.get()
1071+
assert isinstance(result, collections.OrderedDict)
1072+
assert list(result.keys()) == ['123', '100001', '100002', '100003']
1073+
1074+
def test_get_order_by_child_still_sorted_client_side(self, monkeypatch):
1075+
# Guard against overcorrection: ordering by child must still be sorted
1076+
# client-side, since the server returns an unordered collection for it.
1077+
query = self.ref.order_by_child('myValue')
1078+
server_payload = {'k1': {'myValue': 2}, 'k2': {'myValue': 1}}
1079+
self._instrument_body(query, server_payload, monkeypatch)
1080+
result = query.get()
1081+
assert isinstance(result, collections.OrderedDict)
1082+
assert list(result.keys()) == ['k2', 'k1']
1083+
10471084

10481085
class TestSorter:
10491086
"""Test cases for db._Sorter class."""

0 commit comments

Comments
 (0)