Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

FIX: sort strings in UTF-8 encoded byte order #6615

Merged
merged 6 commits into from
Jan 15, 2025
Merged

Conversation

milaGGL
Copy link
Contributor

@milaGGL milaGGL commented Jan 6, 2025

Strings should be sorted in UTF-8 encoded byte order. Public document: https://cloud.google.com/firestore/docs/concepts/data-types#data_types

SDK sorts strings using built in comparator method, which sorts lexicographically, and leads to mismatch between server and sdk when special characters are present. This PR fixes the string order mismatches on document field, map key, and document key.

Copy link
Contributor

github-actions bot commented Jan 6, 2025

Release note changes

The following release notes were modified. Please ensure they look correct.

Release Notes
firebase-firestore
### {{firestore}} version 25.1.2 {: #firestore_v25-1-2}

* {{fixed}} Fixed a server and sdk mismatch in unicode string sorting. GitHub [#6615](//github.com/firebase/firebase-android-sdk/issues/6615){: .external}

#### {{firestore}} Kotlin extensions version 25.1.2 {: #firestore-ktx_v25-1-2}

The Kotlin extensions library transitively includes the updated
`firebase-firestore` library. The Kotlin extensions library has no additional
updates.

Copy link
Contributor

github-actions bot commented Jan 6, 2025

Vertex AI Mock Responses Check ⚠️

A newer major version of the mock responses for Vertex AI unit tests is available. update_responses.sh should be updated to clone the latest version of the responses: v5.3

Copy link
Contributor

github-actions bot commented Jan 6, 2025

Test Results

  186 files  +  144    186 suites  +144   4m 19s ⏱️ + 2m 57s
1 234 tests +  912  1 218 ✅ +  896  16 💤 +16  0 ❌ ±0 
2 492 runs  +1 836  2 460 ✅ +1 804  32 💤 +32  0 ❌ ±0 

Results for commit 4987548. ± Comparison against base commit b5dbd0a.

This pull request removes 322 and adds 1234 tests. Note that renamed tests count towards both.
com.google.firebase.remoteconfig.ConfigTests ‑ Custom Signals builder support multiple types
com.google.firebase.remoteconfig.ConfigTests ‑ Firebase#remoteConfig should delegate to FirebaseRemoteConfig#getInstance()
com.google.firebase.remoteconfig.ConfigTests ‑ Firebase#remoteConfig should delegate to FirebaseRemoteConfig#getInstance(FirebaseApp, region)
com.google.firebase.remoteconfig.ConfigTests ‑ FirebaseRemoteConfigSettings builder works
com.google.firebase.remoteconfig.ConfigTests ‑ Overloaded get() operator returns default value when key doesn't exist
com.google.firebase.remoteconfig.ConfigTests ‑ Overloaded get() operator returns value when key exists
com.google.firebase.remoteconfig.CustomSignalsTest ‑ testCustomSignals_builderPutDouble
com.google.firebase.remoteconfig.CustomSignalsTest ‑ testCustomSignals_builderPutDuplicateKeys
com.google.firebase.remoteconfig.CustomSignalsTest ‑ testCustomSignals_builderPutLong
com.google.firebase.remoteconfig.CustomSignalsTest ‑ testCustomSignals_builderPutMixedTypes
…
com.google.firebase.firestore.AggregateQuerySnapshotTest ‑ createWithCountShouldReturnInstanceWithTheGivenQueryAndCount
com.google.firebase.firestore.AggregateQueryTest ‑ testSourceMustNotBeNull
com.google.firebase.firestore.BlobTest ‑ testComparison
com.google.firebase.firestore.BlobTest ‑ testEquals
com.google.firebase.firestore.BlobTest ‑ testMutableBytes
com.google.firebase.firestore.CollectionReferenceTest ‑ testEquals
com.google.firebase.firestore.DocumentChangeTest ‑ randomTests
com.google.firebase.firestore.DocumentChangeTest ‑ testAdditions
com.google.firebase.firestore.DocumentChangeTest ‑ testChangesWithSortOrderChange
com.google.firebase.firestore.DocumentChangeTest ‑ testDeletions
…

♻️ This comment has been updated with latest results.

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 6, 2025

Coverage Report 1

Affected Products

  • firebase-firestore

    Overall coverage changed from 45.74% (b5dbd0a) to 45.73% (242064a) by -0.01%.

    FilenameBase (b5dbd0a)Merge (242064a)Diff
    LruGarbageCollector.java97.27%93.64%-3.64%
    PatchMutation.java100.00%98.39%-1.61%
    SetMutation.java94.44%97.22%+2.78%
    Util.java73.38%73.94%+0.56%

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/sfp8M5CeVW.html

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Jan 6, 2025

Size Report 1

Affected Products

  • firebase-firestore

    TypeBase (b5dbd0a)Merge (242064a)Diff
    aar1.45 MB1.45 MB+133 B (+0.0%)
    apk (release)11.4 MB11.4 MB+64 B (+0.0%)

Test Logs

  1. https://storage.googleapis.com/firebase-sdk-metric-reports/3X9VBHr9lQ.html

@milaGGL milaGGL marked this pull request as ready for review January 10, 2025 15:57
@milaGGL milaGGL changed the title sort strings in UTF-8 encoded byte order FIX: sort strings in UTF-8 encoded byte order Jan 13, 2025
@milaGGL milaGGL merged commit a1cbf93 into main Jan 15, 2025
33 checks passed
@milaGGL milaGGL deleted the mila/fix-string-comparison branch January 15, 2025 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants