mozilla / application-services

Firefox Application Services
https://mozilla.github.io/application-services/
Other
620 stars 227 forks source link

Bug 1931963 - Add UniFFI bindings for fetch_geonames() in suggest #6483

Closed 0c0w3 closed 1 week ago

0c0w3 commented 2 weeks ago

I didn't expect to be adding bindings for this, and there might be better ways to support the use case described in bug than exposing fetch_geonames() as is, but this works for now at least, and we need a fix for Firefox 134. Desktop will call this to validate cities and states returned by the Yelp ML model. Not only to validate but also to do prefix matching, since we can get that for free.

I considered adding a new function that matches both cities and regions at once so that desktop doesn't need to call into suggest twice and ideally so that we run only one SQL query. But I don't really like the idea of having a completely different code path for this. Or, it might be possible to rewrite the current function so it works that way. But that ends up getting a little messy pretty quickly, and the weather use case doesn't need it. So for now I did the simplest thing.

I also considered making geonames a full-fledged SuggestionProvider and Suggestion, but then we'd have to add more options to SuggestionProviderConstraints to support the fetch_geonames() params, and these aren't really standalone suggestions anyway.

Pull Request checklist

Branch builds: add [firefox-android: branch-name] to the PR title.