Skip to content
New issue

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

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

Already on GitHub? # to your account

Allow keeping state between tile fetches #13

Closed
oherrala opened this issue Oct 20, 2024 · 2 comments · Fixed by #15
Closed

Allow keeping state between tile fetches #13

oherrala opened this issue Oct 20, 2024 · 2 comments · Fixed by #15
Assignees
Labels
enhancement New feature or request

Comments

@oherrala
Copy link

I would like to keep same instance of reqwest::Client between tile fetches. This should alow keeping the connection open between tile fetches and would allow local caching with e.g. http-cache-reqwest crate. In general keeping some kind of state between tile fetches would be useful.

AFAIK the API needs changes to allow this.

Currently I'm trying to do it as:

        let http_client = Arc::new(ClientBuilder::new().build()?);

        let client2 = http_client.clone();
        let fetcher = |x, y, zoom| tile_fetcher(client2, x, y, zoom);
        let snapr = snapr::SnaprBuilder::new()
            .with_tile_fetcher(snapr::TileFetcher::Individual(&fetcher))
            .build()?;

and the compiler error I get is:

error[E0525]: expected a closure that implements the `Fn` trait, but this closure only implements `FnOnce`
  --> mapper/src/lib.rs:20:23
   |
20 |         let fetcher = |x, y, zoom| tile_fetcher(client2, x, y, zoom);
   |                       ^^^^^^^^^^^^              ------- closure is `FnOnce` because it moves the variable `client2` out of its environment
   |                       |
   |                       this closure implements `FnOnce`, not `Fn`
21 |         let snapr = snapr::SnaprBuilder::new()
22 |             .with_tile_fetcher(snapr::TileFetcher::Individual(&fetcher))
   |                                                               -------- the requirement to implement `Fn` derives from here
   |
   = note: required for the cast from `&{closure@smeshty-mapper/src/lib.rs:20:23: 20:35}` to `&dyn Fn(i32, i32, u8) -> Result<DynamicImage, snapr::Error> + Sync`
@c1m50c
Copy link
Owner

c1m50c commented Oct 21, 2024

You can currently (partially) get around this with TileFetcher::Batch if you created the Client inside the passed through function, e.g.

fn tile_fetcher(coords: &[(i32, i32)], zoom: u8) -> Result<Vec<(i32, i32, image::DynamicImage)>, snapr::Error> {
    let client = reqwest::blocking::Client::new();
    todo!("Fetch tiles using `client`");
}

Although, I realize the above example is not ideal for a lot of cases. I do really like the idea of having a more stateful TileFetcher variant, do you think a variant that allows you to pass in an object that implements something akin to a TileFetcher trait would work well for your case? Maybe something like the following?

pub trait IndividualTileFetcher {
    fn fetch_tile(&self, x: i32, y: i32, zoom: u8) -> Result<image::DynamicImage, Error>;
}

pub trait BatchTileFetcher {
    fn fetch_tiles(
        &self,
        coords: &[(i32, i32)],
        zoom: u8,
    ) -> Result<Vec<(i32, i32, image::DynamicImage)>, Error>;
}

pub enum TileFetcher<'a> {
    Individual(&'a dyn IndividualTileFetcher),
    Batch(&'a dyn BatchTileFetcher),
}

@c1m50c c1m50c self-assigned this Oct 21, 2024
@c1m50c c1m50c added the enhancement New feature or request label Oct 21, 2024
@c1m50c c1m50c closed this as completed Oct 21, 2024
@c1m50c c1m50c reopened this Oct 21, 2024
@c1m50c c1m50c closed this as completed in 765b549 Oct 21, 2024
@c1m50c
Copy link
Owner

c1m50c commented Oct 21, 2024

Fixed for real in #15 (v0.5.0), see the "Stateful" example to see how to resolve the issue in a clean way.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants