SwEnt-Group13 / Unio

The world’s largest campus life platform.
3 stars 1 forks source link

Implement address input during Event creation using Nominatim API (with an abstraction level decent enough to switch to other APIs easily) #211

Closed Zafouche closed 3 days ago

Zafouche commented 6 days ago
Zafouche commented 6 days ago

So while implementing this, I ran into an issue, described in this PR, which will most likely be closed. I will quote the problem description:

⚠️ This PR will be abandoned (or not entirely at least)

Problem: Unfortunately, I have found no way of bypassing the requirement of a billing address in the Google cloud services, so this PR will most likely be abandoned. Because of this, I unfortunately cannot see the results of my work, only that it makes the queries to the Places Search API, but the response never returns predictions, only the error message saying that a billing address is required.

Instead

We have to turn to a completely free (and hassle-free) solution. In the bootcamp, we had used Nominatim, but I think it is a poor library, at least, compared to other solutions. After some research, I have found that people prefer the solution of mapbox's Search box API. Try playing around with their playground example. It shows it can fetch really nice informations about locations, and that we can essentially reproduce something similar to the average map apps out there and easy to the eye.

Places Search API is the best API there is out there in terms of location searching. However, it requires to have a billing address. In theory, they give 300$ of credit to use these types of services per account, and it won't charge anything automatically when the entirety credit has been used. However, this is simply not sustainable without funding. So we have to make a choice: Either opt in with Search Places API and use it until the 300$ credit runs out (Note that I have no idea how much time or operations that can amount to. Also, a random bug, if introduced by accident, could use a lot of credit and shorten the time of usage, which is not something we want)

Free solutions

In the bootcamp we had used Nominatim. However it has little support and not much development behind it. The best solution I could find is mapbox's Search box API, which I mentioned in the PR, see the quote above. I think this is the best solution and much more durable without funding (it's free and the rate limits are quite high). We should discuss the matter together, but this is my opinion. Let me know what yours is.

Romainhir commented 6 days ago

I think we need to stick with a free solution. If the "SearchBox" API is not difficult to implement, it could be great to use it. Anyway, go for the easiest one to implement.

Zafouche commented 6 days ago

I think we need to stick with a free solution. If the "SearchBox" API is not difficult to implement, it could be great to use it. Anyway, go for the easiest one to implement.

I think so too. We should discuss this further tomorrow. I have also sent a message to the coaches on discord, check it out

Zafouche commented 5 days ago

For pure geocoding, these are the official OSM alternatives.

Zafouche commented 5 days ago

In terms of pure geocoders, the best seem to be either Nominatim or Photon. We should consider these feature comparisons.

However if we choose to go with a bigger and more complete framework, the likes of mapwork are good. They are built as broader tools, but we could simply just use their Search API. For example, try their geocoding playground, and see the json result for yourself. Essentially, this is pretty similar to Nominatim, so it doesn't make much difference.

The repository and view model for whatever API we use will be the same, as they all work through request URLs, so in the end, it doesn't matter which one we take, we just have to see if the additional details / features one tool gives over the other is preferable.

I will re-start to implement the repositories and view models in a new PR, using one of the tools mentioned above.

Zafouche commented 4 days ago

I will implement our geocoder with Nominatim, while trying to abstract it as much as possible such that it is easy to refactor to the Places Search API if needed. However, this might be hard as the two function very differently: Nominatim uses built URLs, while Places Search API has its own library functions that take care of the queries.

Anywho, a simple implementation of the Places Search API has been done in draft PR #210.

Zafouche commented 4 days ago

I have worked on this further quite a bit. The work is being done in this PR: #224. The URLs are built successfully and return the right results in the JSON. However, I have an issue that prevents the view model from receiving the location suggestions, described below.

Remaining Issues

There is an ongoing issue with how network requests are handled in the NominatimLocationSearchViewModel, specifically related to the use of flatMapLatest (I think). The network requests to the Nominatim API are not completing as expected, possibly due to premature cancellations caused by the current flow logic. This results in location suggestions not being received by the view model.

I have tried debugging the repository, and it looks like none of the code after the following code is never run:

apiService.search(query)

A response is never sent/received, who prevents the view model to receive the location suggestions. Just for information, the URLs are correctly built and the content returned in the JSON from Nominatim is correct.

What remains

Zafouche commented 3 days ago

Never mind the last comment, no issues are actually present. I think the problem was that I was on my home wifi yesterday, and it might be IP banned by Nominatim because of excessive requests (back from when we did the bootcamp, where I probably did an implementation that does not take into account the 1 request per second rate limit).

I was investigating this morning on the EPFL wifi, and I indeed receive the OK packets, so everything works fine! I'll continue onwards to finalize this PR.